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

Fix argument handling in rustic-cargo-clippy-rerun #458

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Kritzefitz
Copy link

This fixes two separate bugs.

The first caused rustic-cargo-clippy-rerun to not pass any effective arguments to rustic-cargo-clippy-run, so the resulting run would always have no arguments.

The second causes rustic-cargo-clippy-rerun to pass rustic-clippy-arguments even when it's not set. This was inconsistent with rustic-cargo-clippy which would fall back to rustic-default-clippy-arguments when rustic-clippy-arguments was empty.

@Kritzefitz
Copy link
Author

Hmm, actually there still seems to be some oddity when rustic-default-clippy-arguments is a buffer-local variable. It gets correctly picked up by rustic-cargo-clippy when run from a file buffer. But it gets weird when these commands are run from the compilation buffer, which might have different buffer-local value for rustic-default-clippy-arguments.

I think oddities like this are to be expected when using buffer-local variables. But I think the situation can be improved by rustic-cargo-clippy always setting rustic-cargo-clippy, even if rustic-default-clippy-arguments were used. rustic-cargo-clippy is less likely to be used as a buffer-local variable and rustic-cargo-clippy-rerun will thus do the right thing more often.

This fixes a bug wgere `rustic-cargo-clippy-rerun` did not pass any
effective arguments to `rustic-cargo-clippy-run`, so the resulting run
would always have no arguments.
This means that running `rustic-cargo-clippy-rerun` would use the same
arguments, even when `rustic-default-clippy-arguments` were used.
@brotzeit
Copy link
Owner

brotzeit commented Nov 19, 2022

Sorry for the late reply. e57139d seems to fix the issue for me, even from a compilation buffer.

I don't see why rustic-default-clippy-arguments is a buffer local variable ? Thanks for the pull request, I would gladly merge it if you revert the second commit, if it works for you. If not, maybe you can give me a list of commands that you used.

@Kritzefitz
Copy link
Author

I have some projects, where I set rustic-default-clippy-arguments in .dir-locals.el as follows:

$ cat .dir-locals.el
;;; Directory Local Variables
;;; For more information see (info "(emacs) Directory Variables")

((nil . ((rustic-default-clippy-arguments . "--all-features"))))

This makes rustic-default-clippy-arguments buffer local for all file buffers in that directory.

@Kritzefitz
Copy link
Author

Oh, I just read your reply again and now I'm confused. You want me to revert the second commit, but you also say that e57139d (which is the second commit) fixed the issue for you. So do you want me to keep the second commit or drop it?

@brotzeit
Copy link
Owner

brotzeit commented Dec 2, 2022

I'm currently a little slow with replying, sorry. I think ab84300 fixed it for me. But if it doesn't work for you we maybe have to take another look.

@Kritzefitz
Copy link
Author

Ok, I see. I still experience problems with only ab84300. You should be able to reproduce them as follows.

  1. Open a rust file in a project and run M-x rustic-cargo-clippy. Clippy will run with the correct parameters specified by rustic-default-clippy-arguments.
  2. Change into the *cargo-clippy* buffer and press g to run clippy again. Note that clippy will be run without any arguments.

This seems like a bug to me. I would expect that the re-run in step 2 would use the same arguments as the first run, but in this case it doesn't. Note that this is different from the behavior when you explicitly override the parameters using M-- M-x rustic-cago-clippy, because in this case the used parameters will be written into rustic-clippy-arguments to be re-used on the next re-run.

e57139d tries to fix this, by always setting rustic-clippy-arguments, so the re-run can always rely on those arguments being present.

An alternative fix could have been to check in rustic-cargo-clippy-rerun if rustic-clippy-arguments is set and if not fall back to to rustic-clippy-default-arguments. But this interacts badly with the possibility that rustic-default-clippy-arguments and would lead to this weird scenario:

  1. Place a .dir-locals.el with the following content in the root of one of your rust projects:

    ((nil . ((rustic-default-clippy-arguments . "--all-features"))))
    
  2. Open a rust file from that project and run M-x rustic-cargo-clippy. Note that clippy will run with the parameters specified in .dir-locals.el.

  3. Switch into the *cargo-clippy* buffer and press g. Note that clippy will run with the parameters from the global value of rustic-default-clippy-arguments, not with the ones used in the last run.

This weird behavior in 3 happens, because .dir-locals.el makes the values specified therein local to all file buffers in that directory. This means, in your rust file it will be buffer-local and take the value from .dir-locals.el, while in *cargo-clippy* it will be the global variable, because its not a file buffer.

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

Successfully merging this pull request may close these issues.

2 participants