rustic icon indicating copy to clipboard operation
rustic copied to clipboard

Fix argument handling in `rustic-cargo-clippy-rerun`

Open Kritzefitz opened this issue 2 years ago • 6 comments

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 avatar Nov 07 '22 20:11 Kritzefitz

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.

Kritzefitz avatar Nov 07 '22 20:11 Kritzefitz

Sorry for the late reply. https://github.com/brotzeit/rustic/pull/458/commits/e57139d4f97ad3800a7307688d4a09e5a3623e66 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.

brotzeit avatar Nov 19 '22 20:11 brotzeit

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 avatar Nov 20 '22 13:11 Kritzefitz

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 e57139d4f97ad3800a7307688d4a09e5a3623e66 (which is the second commit) fixed the issue for you. So do you want me to keep the second commit or drop it?

Kritzefitz avatar Nov 23 '22 16:11 Kritzefitz

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

brotzeit avatar Dec 02 '22 17:12 brotzeit

Ok, I see. I still experience problems with only ab8430055d1e3f683ab81f9f79699652c0ee2722. 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.

e57139d4f97ad3800a7307688d4a09e5a3623e66 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.

Kritzefitz avatar Dec 04 '22 10:12 Kritzefitz