Culture sensitivity

[patricksadowski]

Closes #138

There are still some string concatenation that do not use an invariant output format. But the main points of the issue are fixed.

[drewnoakes]

@patricksadowski thanks for taking the initiative on this. Looks good. Just a few questions to help me understand.

[drewnoakes]

My original review was focussed on how the analyzer works and whether it could be improved.

Thinking about this some more I realise something deeper. #138 is about a bug in parsing where the input is known to be invariant, but parsing code relies upon the current culture.

This PR fixes that, however it also forces all formatted output to use invariant culture, which is not correct. The library should produce output strings that match the user's culture.

If this was merged as-is, all strings would appear in invariant culture (which is essentially en-US).

[patricksadowski]

This PR fixes that, however it also forces all formatted output to use invariant culture, which is not correct. The library should produce output strings that match the user's culture.

Because of your https://github.com/drewnoakes/metadata-extractor-dotnet/issues/138#issuecomment-624027571 I tried to fix the whole issue. I think I have another unterstanding of culture sensitivity. If the app outputs English text then also numbers should be formatted like English numbers: The value equals 1.23 instead of The value equals 1,23 (German). If the library supports multiple languages then text shoud be available in each supported language: The value equals 1.23 and Der Wert ist gleich 1,23

To get this done I propose to fix only the main bug of the issue. Your https://github.com/drewnoakes/metadata-extractor-dotnet/issues/138#issuecomment-624027571 should be done in a second PR oder issue. Is it OK for you?

[drewnoakes]

Because of your comment I tried to fix the whole issue

Yeah sorry, I hadn't thought the implications through when I wrote that. It wasn't intended as a strong statement. It may be possible to configure the analyzer to do what we want (via attributes or patterns in the .editorconfig file).

If the app outputs English text then also numbers should be formatted like English numbers

This is debatable. I'm pretty sure if this were changed then we'd see bug reports. Ideally all output strings would be localised.

To get this done I propose to fix only the main bug of the issue.

That's certainly the safest way forward here. Perhaps we add a bit more context to the TODO in the .editorconfig file too.

[drewnoakes]

/azp run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).

[drewnoakes]

Thanks very much Patrick.