Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature proposal: pass arguments to overriden commands in a json to make them more flexible and use other rust-analyzer settings #13258

Closed
IceTDrinker opened this issue Sep 19, 2022 · 3 comments

Comments

@IceTDrinker
Copy link

Hello,

My proposal is something that would break existing overriden command which I believe take the repo root as sole argument.

The use case is the following:

rust-analyzer.cargo.buildScripts.overrideCommand cannot use other settings from rust analyzer (like enabled features e.g.) as the overrideCommand will be executed as is and will only get the root of the current repository as argument.

My proposal is to pass all the arguments that rust analyzer would usually use for building its own build script command (or even dumping it's whole settings as json, unsure if it's useful) as a json to the custom command.

This allows the user writing a custom command to write a sort of generic entry point that can use settings from rust-analyzer.

Schematically:

current:

rust-analyzer.cargo.buildScripts.overrideCommand = ["my_command"]

would launch my_command /path/to/repo/root

proposal:

rust-analyzer.cargo.buildScripts.overrideCommand = ["my_command"]

would launch something like my_command '{"enabled_features": ["feature_1", "feature_2"], "repo_root": "/path/to/repo/root"}' leaving the choice to my_command to parse the json and build a custom command that could be closer to the default rust-analyzer command in terms of configuratibility/versatility.

Don't know if there is a big need for this, but it feels like overriden command could benefit from more flexibility at this level 🙂 also if people go to the length of writing custom commands giving them the choice to do more with their custom commands does not feel out of place.

Cheers!

@Veykril
Copy link
Member

Veykril commented Sep 19, 2022

I agree that the current overrideCommand is rather inflexible and there is room for improvement, but I'd much rather experiment with some form of interpolation for it instead. Passing a string that contains a json blob of information seems incredibly unwieldy.

In fact, #13128 will most likely implement interpolation for manifest-path, and with that probably the basics for interpolation for that in general.

@IceTDrinker
Copy link
Author

Oh yeah interpolation was definitely a possitibility, it seemed just a bit more complicated and constrains the form of the call a lot, that's why I was thinking json dump 🙂

@Veykril
Copy link
Member

Veykril commented Sep 19, 2022

Passing a json blob would complicate the job for the callee by a lot, as then the command has to parse json which is not a trivial task. Anyways I would very much prefer the interpolation option, which should cover 99% of the use cases. The other 1% I would wonder what they are trying to do, if we can't expose the needed information via interpolation.

@Veykril Veykril closed this as completed Sep 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants