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

No way to pass multiple values to list-like --config options #6097

Open
jaybosamiya opened this issue Feb 26, 2024 · 1 comment
Open

No way to pass multiple values to list-like --config options #6097

jaybosamiya opened this issue Feb 26, 2024 · 1 comment
Labels
only-with-option requires a non-default option value to reproduce

Comments

@jaybosamiya
Copy link

There appears to be no way to pass multiple values to a configuration parameter such as skip_macro_invocations from the command-line:

$ rustfmt --config 'skip_macro_invocations=["foo","bar"]'
invalid key=val pair: `skip_macro_invocations=["foo"`

$ rustfmt '--config=skip_macro_invocations=["foo","bar"]'
invalid key=val pair: `skip_macro_invocations=["foo"`

I believe this is due to the immediate split of the argument done here:

rustfmt/src/bin/main.rs

Lines 572 to 577 in 21f353a

options.inline_config = matches
.opt_strs("config")
.iter()
.flat_map(|config| config.split(','))
.map(
|key_val| match key_val.char_indices().find(|(_, ch)| *ch == '=') {

jaybosamiya added a commit to verus-lang/verusfmt that referenced this issue Feb 28, 2024
Rustfmt messes with verusfmt's formatting, and this has been the cause of idempotency issues such as #33 and #36. With this PR, we close that out (hopefully) entirely.

In short, we add a new (minimal) parser that collapses any `verus!{ ... }` into `verus!{}` before passing it to rustfmt; upon getting a result from rustfmt, we expand out those `verus!{}` to `verus!{ ... }` again, thereby preventing rustfmt from messing with anything inside the body that we have carefully set up already.

This also allows us to enable the NR snapshot test, which is now stable. Fwiw, we could have fixed NR's non-idempotency if we could pass multiple macros to `rustfmt`'s `skip_macro_invocations` config; however, that is blocked on an issue in rustfmt: rust-lang/rustfmt#6097; even once that is fixed, we would need to assume/check that the user is on recent-enough rustfmt (also it turns out that `skip_macro_invocations` is not stable on rustfmt), so the fix that this PR does is a much more manageable fix for us overall.

Additionally, this PR also adds in vstd as a snapshot test (which could not be added before, due to same idempotency issue).

Fixes #36
@ytmimi
Copy link
Contributor

ytmimi commented Mar 5, 2024

Thanks for the report. Going to link to the tracking issue for skip_macro_invocations #5346

@ytmimi ytmimi added the only-with-option requires a non-default option value to reproduce label Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
only-with-option requires a non-default option value to reproduce
Projects
None yet
Development

No branches or pull requests

2 participants