dotnet / msbuild

The Microsoft Build Engine (MSBuild) is the build platform for .NET and Visual Studio.
https://docs.microsoft.com/visualstudio/msbuild/msbuild
MIT License
5.21k stars 1.35k forks source link

Multiword configuration options renaming #10315

Closed JanKrivanek closed 1 month ago

JanKrivanek commented 1 month ago

Context

.editorconfig standard allows only lowercase names. For this reason multiword options usually use underscores to separate multiple words within same option (e.g. csharp_space_after_keywords_in_control_flow_statements).

In our implementation and documentation we are using camel-casing.

Goal

f-alizada commented 1 month ago

Could you please clarify if the same applicable to the custom configuration data? Example: if I specify the my_test_data in the editor config I expect find my_test_data in the custom configuration dictionary. Or the goal is applicable only for the configuration recognized by the infrastructure?

JanKrivanek commented 1 month ago

I would not manipulate custom config data at all - let's just ensure they are case insensitive (which I believe you already did).

For the infra-recongnized config keys - let's check with @baronfel what syntax feels preferrable. E.g. for EvaluationAnalysisScope configuration (within our code - only going to be added soon) - do we want that to be exposed in .editorconfig as evaluation_analysis_scope or anyhow differently?

f-alizada commented 1 month ago

It was discussed offline, great idea from @baronfel is to simplify the usage of the evaluation_analysis_scope for future is to reference it from the configuration/editorconfig by the scope key :)

f-alizada commented 1 month ago

@JanKrivanek I would like to point out to the 1st Goal in the list

Configuration module should be able to translate options_with_underscores to C# OptionsInCamelCase.

Is there a chnace you could clarify the use case for that?

The editorconfig is closely related to the user facing scenarios, hence I would like to propose to decouple the variable namings defined in the code from the configuration.

Meaning that what was suggested if we have the scope key defined in the configuration it is mapped (1-1) to the EvaluatioAnalysisScope. Something like that: https://github.com/dotnet/msbuild/pull/10361/files

JanKrivanek commented 1 month ago

@JanKrivanek I would like to point out to the 1st Goal in the list

Configuration module should be able to translate options_with_underscores to C# OptionsInCamelCase.

Is there a chnace you could clarify the use case for that?

The editorconfig is closely related to the user facing scenarios, hence I would like to propose to decouple the variable namings defined in the code from the configuration.

Meaning that what was suggested if we have the scope key defined in the configuration it is mapped (1-1) to the EvaluatioAnalysisScope. Something like that: https://github.com/dotnet/msbuild/pull/10361/files

The current implementation proposal looks great: https://github.com/dotnet/msbuild/pull/10361/files#diff-889c51f15e7c4415667244aaecf9a9c77442af8baefec35fe30a95ac3425915cR85-R93

As soon as we do not mandate casing nor multiword keys without separation (e.g. 'someconfigurationkey') within editorconfig - it's completely internal detail what will be the internal configuration keys within our code and how the mapping will work.

Let's just make sure the documentation is up to date with what we support in editorconfig (which we might already have updated)

tl;dr; - there might actually by no work already :-)

f-alizada commented 1 month ago

Conclusion: Having the infrastructure that allows use any configuration values and keys from the defined in the code, the goal defined in the issue:

Configuration module should be able to translate options_with_underscores to C# OptionsInCamelCase.

is solved. There is no need to have this possibility in the configuration provider to translate and map, but just map.

Please feel free to reopen if needed