MBulli / SmartCommandlineArgs

A Visual Studio Extension which aims to provide a better UI to manage your command line arguments
GNU General Public License v2.0
99 stars 35 forks source link

Add Custom JSON Root Path Setting #146

Closed orbikm closed 1 year ago

orbikm commented 1 year ago

(Rebased onto master from #145, feature/vs2022/json-root-path)

This feature implements behavior for allowing a user to specify a custom root path for all JSON settings for each project. This works by allowing the user to specify the Root path, and the Solution name and Project names are added to the path.

This allows for saving modified Command Line arguments in a Source Control solution, while having an ephemeral Visual Studio project (such as the case when the VS projects are generated by a build system). Due to this, this resolves issue https://github.com/MBulli/SmartCommandlineArgs/issues/115

A description of how we would plan to use this in practice would be something like:

  1. Have our build system generate a SolutionName.ArgsCfg.json that includes an entry for the Custom JSON Root, which is known by the build system
  2. The Custom Json Root folder will be checked into source control, allowing development teams to add useful sets of command line arguments. These changes, since they're directly pointing from Extension to the custom path, will allow devs to save and commit JSON modifications
Irame commented 1 year ago

Thanks for the rebase! I had a closer look at it now and have one general question. Is it necessary to create so many subfolders? In my opinion the "Use Solution Directory" option should be forced active, which could be renamed to "Use Single File". Then you only have one file per solution and can avoid having different subfolders inside the custom JSON Root.

Let me know what you think! (I can also merge the PR and implement this myself, if you don't mind.)

orbikm commented 1 year ago

Hrm, I wasn't really aware of that as an option. I think I like it, but do you think there'd be a benefit towards allowing either approach? As in, don't force the "Use Single File" option, but they could work in conjunction? As in, if they have Use Custom Json Root on and a root path chosen, and they don't select "Use Single File", it would operate as it is now. If they do, then it would collapse everything into a the custom JSON root under the solution name for all the projects.

I'm honestly fine with either. I think personally while integrating this extension into our workflow, I'd likely enable the Use Single File as well, so the directories are a little cleaner anyways, so whatever you think is the right approach, I'd be happy with.

If you wouldn't mind doing the implementation and merge yourself, that'd be much appreciated. There's no guarantee I'll have the time this weekend / week to get this accomplished.

Thanks for your collaboration!

Irame commented 1 year ago

FIY: I merged the PR in a separate branch for now. You can take a look and see if this works for you.

The JSON file your build system has to generate/copy for the settings should look something like this:

{
  "SaveSettingsToJson": true,
  "VcsSupportEnabled": true,
  "UseSolutionDir": true,
  "UseCustomJsonRoot": true,
  "JsonRootPath": "..\\args"
}

I didn't change UseSolutionDir to UseSingleFile yet, because then I would want to change the name in the JSON as well. That would introduce some changes for existing users, and I have to think about that.

orbikm commented 1 year ago

As for the UseSolutionDir / UseSingleFile, you could opt to read in the order of UseSolutionDir first, and then can be overridden by UseSingleFile, as a means of supporting legacy code, but ultimately respecting the new variable.

If you are concerned about schema changes like this, I would highly recommend you add a Version field to the JSON as well, so you can make decisions based on the specific version of the JSON file you're reading.

Irame commented 1 year ago

Hi and thanks for the feedback.

In general I would do the same. I event had it implemented already but then reverted it again. There are two reasons why I decided not to do it. Firstly, it would introduce unnecessary Git changes. Secondly, it could lead to issues if there are multiple people working with the same git repository but have different versions of this extension.