feat: split wrapper command on linux

[Rayzeq]

This allows specifying multiple commands, and arguments, as the wrapper command (i.e gamemoderun mangohud --dlsym) without having to create a separate script.
Note: this will break for people who have a space or quotes in their wrapper command (which could happen if they have a custom script with a space in the path)

[AlexTMjugador]

A PR for this has already been opened, but it was not merged because its author didn't end up addressing review comments I made there: #3900

I think it'd be best if you continued the work done there to get it to a mergeable state. Is that okay with you, @webbgamers?

[Rayzeq]

I assume I can't just add commits to #3900, so I guess I would need to start from that pull request, and put its content and the fixes in this one ?

[AlexTMjugador]

I assume I can't just add commits to #3900, so I guess I would need to start from that pull request, and put its content and the fixes in this one ?

Yes, that would work. You can push the commits of that PR branch to this one, and then work on top of that. That PR is a bit old though, so you'd have to resolve some merge conflicts first while updating it to work with the latest commits.

[AlexTMjugador]

Alternatively, if @webbgamers gives you push permission to their branch, you may also just work in #3900.

[webbgamers]

Either works for me. The only thing left to do really is to come up with a global settings migration system to handle transferring global hooks from the old to new parsing. Such a system already existed for migrating profiles but will need to be added to the global settings too. That will probably involve adding a new version column to the settings table and creating a sqlx migration to add it on existing databases.

We could also just not change the global arg parsing and get improved profile hook parsing merged, but that would make hooks behave inconsistently between global and profile settings.

[AlexTMjugador]

Awesome, thanks a lot for your comment @webbgamers! Both of you, please free to use this PR comment thread or our Discord server to coordinate anything that needs to be done here. Ideally, we'd credit both of you as contributors when merging whatever PR! 😄

[Rayzeq]

I changed this pull request to use code from #3900 (directly copying the code was easier than using the commits from @webbgamers, I hope it's okay with him).

However, as per my initial PR, this PR will not change the behavior on Windows systems for the following reasons:

1. Splitting arguments on windows only seems possible by directly calling CommandLineToArgvW from the winapi (which has no safe wrapper). Another solution is to use cmd.exe /C <command>, but it would have different behavior than the current implementation because cmd.exe parses command lines differently than process::Command, especially when handling batch files.
2. The only crate I could find for quoting arguments on windows is shell_escape, which quotes for cmd.exe and not process::Command.

So, if we want to implement this on Windows, the most sensible option is probably to just use shlex, but it may break some user's hooks, and I'm not sure if that's acceptable.

[webbgamers]

I have not been able to think of a string that would be broken given that the previous behavior was either to split by spaces or to pass the entire string to Command::new. From my testing, Windows and Linux paths were correctly quoted by shlex::quote such that shlex::split resulted in the original string being passed to Command::new. I don't think it is necessary that strings entered on a Windows install need to be parsed exactly how cmd.exe would parse them. It might be worthwhile to add a note on the hooks window that hooks will be parsed following Unix standards.

[AlexTMjugador]

I agree with @webbgamers here. We don't need to follow OS conventions for argument parsing, especially given that the Modrinth hooks configuration is technically cross-platform (you can copy the configuration database between operating systems and it should still work, though realistically I have not tested it and it's more of a neat property to have than a hard guarantee we're making). Unix standards just happen to be simpler, consistent and straightforward enough for the audience this feature targets.

Breakage in any currently configured hooks due to the different argument splitting should be prevented via a migration-like mechanism as I described in #3900, so there are no concerns from that side either.

[webbgamers]

#3900 and now this PR both include migration for profile specific hook settings. The only remaining step is to create a migration mechanism for global settings and implement the migration for global hook settings. The migration mechanism for profiles provides a good starting point but might need more robust error handling since messing up global settings is worse than messing up a profile.