use rustic-test-arguments by default

[taquangtrung]

Hi,

Currently, the command rustic-cargo-test always runs with the default argument rustic-default-test-arguments. Users can set up a custom argument using C-u, but this is only applicable for the current run, and next time, users need to run C-u again to update the argument.

I fixed this issue by enabling rustic-cargo-test to run with rustic-test-arguments, which is initialized by rustic-default-test-arguments, and is only needed to be updated by users once using C-u.

Can you review and merge this PR if possible?

Thanks!

[CeleritasCelery]

I agree this is something that needs to be fixed. The problem is really that the behavior is not consistent between using the popup and stand-alone command. The way the popup behave gives you the behavior you want (using rustic-test-arugments by default). I would say this simplest way to fix this would be to unify the two cases.

rustic/rustic-cargo.el

Lines 173 to 180 in 4879922

	(cond (arg
	(setq rustic-test-arguments (read-from-minibuffer "Cargo test arguments: " rustic-default-test-arguments)))
	((eq major-mode 'rustic-popup-mode)
	(if (> (length rustic-test-arguments) 0)
	rustic-test-arguments
	rustic-default-test-arguments))
	(t
	rustic-default-test-arguments))))

I imagine it looking like this

(cond (arg
       (setq rustic-test-arguments (read-from-minibuffer "Cargo test arguments: " rustic-test-arguments)))
      ((string-empty-p rustic-test-arguments)
       rustic-default-test-arguments)
      (t rustic-test-arguments))

[brotzeit]

The idea was that you can use the rerun commands if you want to reuse the stored arguments. I'm not sure why I decided to make this behavior different for the popup, maybe so you don't have to add two commands to the popup menu.
If we apply this change the rerun commands wouldn't be needed anymore and you would have to manually reset rustic-test-arguments if you want to use the default arguments again. There are probably people that got used to the way it works now.

Would it be ok for you if we replace the popup condition with a new option.

(cond (arg
          (setq rustic-test-arguments (read-from-minibuffer "Cargo test arguments: " rustic-default-test-arguments)))
         (rustic-cargo-always-use-stored-arguments ;; maybe better name ?
          (if (> (length rustic-test-arguments) 0)
              rustic-test-arguments
            rustic-default-test-arguments))
         (t
          rustic-default-test-arguments))

[taquangtrung]

@brotzeit: in my patch, to reset rustic-test-arguments, users can just need to run rustic-cargo-test with C-u. It will prompt a question to reset that variable to rustic-default-test-arguments:

(setq rustic-test-arguments (read-from-minibuffer "Cargo test arguments: " rustic-default-test-arguments))

Nevertheless, I agree with you that this functionality is not straightforward and my patch also changed the current default behavior of rustic-cargo-test.

+1 vote for the use of option using rustic-cargo-use-stored-arguments.

Maybe change the name to rustic-cargo-use-last-stored-arguments or rustic-cargo-use-last-used-arguments?

[brotzeit]

@CeleritasCelery What do you think ?

[CeleritasCelery]

I like the idea of making it a user option for preserving the current behavior if desired. We also probably want to update rustic-cargo-run to have the same behavior for consistency.

And my personal preference for bikeshedding over the variable name would be rustic-cargo-reuse-arguments 😄

[CeleritasCelery]

@taquangtrung Do you still want to implement this?

[taquangtrung]

@CeleritasCelery: seems that @brotzeit already implemented it in this commit: 064aeef