search

spice2x / spice2x.github.io

🌶️ spice2x is a spicier fork of SpiceTools with hundreds of bug fixes and new features 🌶️
https://spice2x.github.io/
GNU General Public License v3.0
117 stars 2 forks source link

Command line option should have precedence #190

Open guardianblue opened 4 months ago

guardianblue commented 4 months ago

As title. When specifying an option through command line, it should overwrite existing configuration specified through spicecfg.

Likewise, if the same option is specified multiple times in command line, the last occurrence should take effect, although this is an edge-case.

This would speed up development and come in handy for users who connect to multiple servers.

sp2xdev commented 3 months ago

I agree this should have been the default behavior since day 0, but unfortunately this isn't the case.

One big risk with making a change like this is breaking people with existing setups.

I don't know what the current behavior is when there are conflicting flags (will really need to look into it soon...), but imagine a scenario:

  1. User sets -url mylocalserver:8080 in spicecfg and -url reallystrictn-1server:8080 in batch file (i.e., set this up but just forgot about it)
  2. In old spicetools/spice2x version, running this batch file connects to mylocalserver
  3. Update to latest spice2x with change to have command line take precedence
  4. User launches batch file again, accidentally connects to reallystrictn-1server and gets banned

Maybe better to just exit and spit out an error for some key options (server URL, pcbid, cardid) if there are conflicting parameters.

guardianblue commented 3 months ago

Backward compatibility is a fair concern. The reason I suggested this change is mainly for debugging / development purpose, and I would say normal users will probably not have several batch files to start spice2x in different options sets.

With that said, I think what can be done here is to add another flag, say -cmdoverride, for this new behaviour. Existing users won't have this flag on while developers will know what they are doing when using the flag.

daijoubussy commented 1 month ago

maybe handle this as a breaking change and do it while migrating to a json-based config? getting out of appdata in general is something that makes sense with dir-based configs being the expected norm imo.

sp2xdev commented 1 month ago

I'd rather not do any breaking changes. Compatibility with old spicetools is still a stated goal.