Make sort sensitivity opt-in and configurable #41
Conversation
| @@ -63,5 +162,24 @@ describe('SortByKeyPostProcessor', () => { | |||
| i: { value: 'letter i', sourceFiles: [] }, | |||
| j: { value: 'letter j', sourceFiles: [] } | |||
| }); | |||
|
|
|||
| expect(Object.keys(processor.process(collection, new TranslationCollection(), new TranslationCollection()).values)).to.deep.equal( | |||
| Object.keys({ | |||
There was a problem hiding this comment.
I don't understand what this assertion is testing beyond what the test above it is testing? Can you clarify this?
There was a problem hiding this comment.
According to my findings, the to.deep.equal does not assert the order in Objects.
These tests were added quite recently by this PR: #35 but these these have not been designed to verify the order of the extracted keys.
I modified the assertion a bit and added some comments to make this more clear
Additionally, some tests were added to have at least one assertion per sort-sensitivity option
README.md
Outdated
| @@ -110,6 +110,7 @@ Output | |||
| --format, -f Format [string] [choices: "json", "namespaced-json", "pot"] [default: "json"] | |||
| --format-indentation, --fi Format indentation (JSON/Namedspaced JSON) [string] [default: "\t"] | |||
| --sort, -s Sort strings in alphabetical order [boolean] | |||
| --sort-sensitivity, -ss Sensitivity when sorting strings [string] | |||
There was a problem hiding this comment.
I think it would be good to add a dedicated section to the readme about sorting now. If I understand correctly, the -ss flag only has an effect in combination with the -s flag right? That should be noted.
Also this section can explain how strings will be sorted without the -ss flag, as well as what the different options are and what effect they have on the sort order.
There was a problem hiding this comment.
Limitations for the -ss flag have been added, as well as a separate section in the README
|
Thanks for the feedback. I pushed some changes to my initial PR with the proposed additions. |
|
Thank you! |
|
This feature was just published in v9.2.0 👍 |
In a recent PR (#35), the sorting of strings was adjusted to be case-insensitive. This is not an option for us, so I made the functionality optional with the parameter
--sort-sensitivity. If not provided, the old behavior is re-applied.The unit tests for the postprocessor so they have been updated and expanded