krakenjs / spud

A content store parser, reading a java .properties-like format
Other
14 stars 9 forks source link

Support "trim" for value, ie remove leading and trailing whitespace #21

Closed vladimiry closed 9 years ago

vladimiry commented 9 years ago

I'm using spud to convert .properties to .json.

For example this value

Answer_Translated_Understand = Do you understand this translation?

Converted to

Answer_Translated_Understand": " Do you understand this translation?",

I don't want to preserve left/leading space and I can't edit *.properties to set = without spaces (like this "Answer_Translated_Understand=Do you understand this translation?"). So it would be great to get values without spaces at left/right, perhaps as optional feature.

aredridel commented 9 years ago

Is there a reason you can't postprocess to do this?

vladimiry commented 9 years ago

Sure, this would be an extra code.

I'm using in this way:

    // convert *.properties to *.json
    spudDefers = i18nConfig.properties.map(function (item) {
        var deferred = Q.defer(),
            srcPath = path.join(i18nConfig.propertiesDir, item.file + '.properties'),
            destPath = path.join(i18nConfig.destDir, item.lang + '.json'),
            rs = fs.createReadStream(srcPath),
            ws = fs.createWriteStream(destPath);

        spud.convert(rs, 'properties', 'json', ws, function (err) {
            if (err) {
                return deferred.reject(err);
            }
            console.log('File', '"' + srcPath + '"', 'converted to', '"' + destPath + '"');
            deferred.resolve(true);
        });

        return deferred.promise;
    });
aredridel commented 9 years ago

Ah! you're running it as conversion process. I was in the process of ripping that out for spud version 2. The feature makes more sense in that context since you never touch the content yourself.

aredridel commented 9 years ago

I'd accept a patch to do this optionally in spud version 1 if you want, though do be aware that moving forward, spud version 1 will be deprecated in favor of version 2.

vladimiry commented 9 years ago

Yep, I would not want to read already processed json file modify and write again.

vladimiry commented 9 years ago

@aredridel where can I get v2 to take a look? Currently I'm using this module https://www.npmjs.com/package/spud v1.1.2

aredridel commented 9 years ago

https://github.com/krakenjs/spud -- the default branch is v2.x

vladimiry commented 9 years ago

Thanks, but no npm/bower package so far?

aredridel commented 9 years ago

npm install spud@2

aredridel commented 9 years ago

'dist-tags': { latest: '1.1.2', minimal: '2.0.1' },

aredridel commented 9 years ago

2.x will become latest dist-tag when the things that depend on it are stable.

vladimiry commented 9 years ago

Thanks, I will review v2 features.

aredridel commented 9 years ago

Excellent. It has a very simple, JSON-like API: just a parse and stringify method. I figured file reading was better left to node, which has good APIs for this already.

vladimiry commented 9 years ago

Yes, I see how this works https://github.com/krakenjs/spud/blob/v2.x/test/properties.js Well I can use v2 actually, will be a bit more code but I will have full control. So I think this issue could be closed.

aredridel commented 9 years ago

Awesome!

diegone commented 9 years ago

I actually think it'd be a best practice to trim strings in resource files. Developers may be careful adding spaces before and after, but after the files are translated into 20 different languages, linguists may not be as careful (or aware) and you open yourself up to a lot of l10n bugs because a linguist or a machine stripped or added a leading/trailing space.

aredridel commented 9 years ago

Definitely -- which is why a low-level tool like this should probably stay uninvolved, so that a higher-level tool can do those checks.

vladimiry commented 9 years ago

@diegone > I actually think it'd be a best practice to trim strings in resource files.

Not exactly, the resource file parsers should not recognize leading/trailing space at all, this is standard behavior on Java at least and this is good. If there is need in such spaces it could be done using Unicode escape sequence \u0020 or in code logic. That is why I created this issue - parser processes resource files is not quite the standard way (preserves leading/trailing). But now I see v2 and will use it.

diegone commented 9 years ago

But v2 still preserves the space, see keyValueTest2: https://github.com/krakenjs/spud/blob/v2.x/test/properties.js#L23

vladimiry commented 9 years ago

Yes, it's, but using v2 I will have result in form of object so will able to post-process it before write to file.