If apply_single refers to some non-existent action, just nothing happens. The function just returns with True:
This allowed hacks like setting apply_single=True in the 2nd variant of ConsoleInteractionTest.test_acquire_actions_and_apply (which is getting properly renamed by https://github.com/coala/coala/pull/4860 BTW), which then causes further problems in PRs like https://github.com/coala/coala/pull/4179#discussion_r150409980
The above test needs anyway some redesign according to #4862 whereby apply_single=True should just not be used
ask_for_actions_and_apply needs to raise some exception on invalid apply_single values
cc @mrtes @Nosferatul @Adrianzatreanu @yukiisbored
We could use the existing action types, instead of magic strings.
That way we have encoded/documented which values are allowed for apply_single, and can handle other input properly.
I would remove the magic description strings in ask_for_action_and_apply in general.
Instead we could link the descriptions to the action types via constant fields (instead of function docstrings) and use these constants in comparisons, if necessary.
Additionally, the letter that maps to the action (N for Do (N)othing), could be stored in this way as well.
With some other tweaks (e.g. a dictionary that looks up actions by letter), the complexity of the action choosing code could be reduced.
Hey, this may be caused by our improve CLI project. Looking back, I think we should've done a rewrite on this to fit the new behavior.
@yukiisbored It was caused by that ;) But we can still do some kind of rewrite... @mrtes seems also motivated
As time permits ;)
The whole apply_single business looks to me like a non-interactive run, with a defined default action, which is queried by user input (in run_coala).
Interestingly it is only queried if a respective command line arg is present.
Then coala is run normally, but due to the apply_single parameter, no user input is requested in ask_for_action_and_apply. Instead, the action matching apply_single is applied.
Interestingly, it wont be applied in an actual non-interactive run, which is confusing me.
In my opinion we could move the choice of apply_single to the command line and just error out, if it does not match an action.
That would make it useable in a non-interactive run.
But maybe I'm misinterpreting the intentions of the apply_single arg and the non-interactive mode.
I would like to work on this :)
It may have been fixed in some commit, returns False now. Line
Most helpful comment
We could use the existing action types, instead of magic strings.
That way we have encoded/documented which values are allowed for
apply_single, and can handle other input properly.I would remove the magic description strings in
ask_for_action_and_applyin general.Instead we could link the descriptions to the action types via constant fields (instead of function docstrings) and use these constants in comparisons, if necessary.
Additionally, the letter that maps to the action (N for Do (N)othing), could be stored in this way as well.
With some other tweaks (e.g. a dictionary that looks up actions by letter), the complexity of the action choosing code could be reduced.