Use modules properly and allow composability #44
Conversation
jdesboeufs
changed the title
|
This is awesome and super needed!!! Thank you so much for doing it, I always wanted to organize it this way but could never find the time. |
| var consts = jschardet.Constants; | ||
|
|
||
| jschardet.HZ_cls = [ | ||
| exports.HZ_cls = [ |
There was a problem hiding this comment.
HZ_cls and a few more are never used outside of this scope so we don’t really need to add it to exports. Not sure why I added it to jschardet at the time, but we can avoid it now.
| @@ -27,16 +27,19 @@ | |||
| * 02110-1301 USA | |||
| */ | |||
|
|
|||
| !function(jschardet) { | |||
| var CodingStateMachine = require('./codingstatemachine') | |||
There was a problem hiding this comment.
We can also use this reorganization to make the filenames camel case, so they match their symbols: CodingStateMachine.js.
| @@ -39,13 +39,13 @@ Options | |||
| ```javascript | |||
| // See all information related to the confidence levels of each encoding. | |||
| // This is useful to see why you're not getting the expected encoding. | |||
| jschardet.Constants._debug = true; | |||
| jschardet.setLogger(); | |||
There was a problem hiding this comment.
This name is not clear of its purpose. I had to read the setLogger code to figure out what it was doing.
My first reaction was to think that this wouldn’t do any logging because you were setting the logger to nothing.
Let’s add two functions: {enable, disable}Logging().
| var setLogger = require('./logger').setLogger; | ||
|
|
||
| exports.VERSION = "1.4.1"; |
|
|
||
| // Default minimum accepted confidence level is 0.20 but sometimes this is not | ||
| // enough, specially when dealing with files mostly with numbers. | ||
| // To change this to 0 to always get something or any other value that can | ||
| // work for you. | ||
| jschardet.Constants.MINIMUM_THRESHOLD = 0; | ||
| jschardet.detect(str, { minimumThreshold: 0 }); |
| return u.result; | ||
| } | ||
| exports.UniversalDetector = UniversalDetector; |
There was a problem hiding this comment.
We don’t need to export this I believe.
| exports.log = function () {}; | ||
|
|
||
| exports.setLogger = function setLogger(fn) { | ||
| exports.enabled = true; |
There was a problem hiding this comment.
Instead of exporting this and add the check everywhere, we can just make the check on the log function itself (at line 10).
|
@aadsm is there an ETA on this? as this is very important to allow static compilation, like with |
|
@jdesboeufs I had forgotten about this, I'll try to take a look over the weekend. |
|
I made some changes and I just published a new version with this. |
Hi,
I needed to detect encoding in a small frontend tool.
This library is awesome, but too huge for my use case.
I started to investigate how to cut it down.
Sometimes, it is not necessary to be able to detect all encodings available in
jschardet.In my case, we develop a tool for French people, so we only need to support Windows-1252/Latin-1 and UTF-8.
This PR is a code reorganization, using modules the right way (with their own dependency tree).
It's a first step before going further!
For now, it breaks compatibility for those who use actual
_debugandConstants.MINIMUM_THRESHOLD. I updated the README.This reorganization make possible writing custom detectors.
It's at the moment very quick and dirty, but if we work together I'm sure we can do a better
jschardet2.0:)See example custom code:
jdesboeufs/jschardet@use-modules...jdesboeufs:fr-cut
See also my independant PR #42 then #43.