Command line: allow to not set input plugin for code

[unkcpz]

For feature #4991.

An extra flag option --bind-to-input-plugin with default flag to True is add to verdi code setup to allow not to set the input plugin for the code.
This may increase the configuration complex a bit of code setup. Especially when setting code through config file, if user do not want to bind input plugin to the code they need to set this flag to false explicitly like:

---
label: {label}                    
bind_to_input_plugin: no
computer: {computer}
remote_abs_path: /remote/abs/path

[sphuber]

Thanks @unkcpz . I agree that we should make this input optional, since it is a default value after all. But why don't we just make the option truly optional? In that case, a user simply does not provide a value for the default plugin prompt. Does this approach not work somehow?

[unkcpz]

It already a optional option since the default value of required is required=False, and it works quite well with setting code in non-interactive way. The problem comes with set code interactively, what you expect users to input? Since it's not possible to just click the ENTER and continous for setting the following options.

There are following chooses(but I think with their own downside) to do it without the extra flag I set it here:

1. Allow user to input None to not set this when the option prompt out.
2. The default value is None and user can type Enter and go ahead.
3. use CHARACTER_IGNORE_DEFAULT ! as the input, but I guess the ! only for TEXT option such as append_text and prepend_text?

Moreover, input_plugin option read a PluginParamType which is a subclass of NonEmptyStringParamType, so it is not easy to convert None or empty string for it.

[sphuber]

I think this is rather just a bug in the behavior. The option is required, so a None value is perfectly acceptable. Which means that if the user is prompted but presses ENTER, then that should simply be acceptable as setting None. I would rather fix that behavior than complicate the interface with another dependent option.

[unkcpz]

Thanks!

I would rather fix that behavior than complicate the interface with another dependent option.

I agree that it is a bad idea to complicate the interface. Thanks for tell me this is a bug, I thought it was designed to being like that, to prevent user from type ENTER without thinking too much etc.
I'll rollback and fix it in the way you recommended.

[sphuber]

I am not sure yet why the prompt keeps prompting even if None should be accepted, however, it might have to do with our InteractiveOption. It hacks click's prompt cycle quite a bit and may have made a mistake. Note that I have an open PR to upgrade to click==8.0 which gets rid of most of the custom logic, since the prompt logic in click was largely reworked. Please have a look there as well, since fixing this issue may have to do with the changes there as well.

[unkcpz]

Hi @sphuber, I believe the mentioned repeat prompt behavior is by design. I find a test case to check this:

aiida-core/tests/cmdline/params/options/test_interactive.py

Lines 148 to 158 in b31e5f5

	def test_prompt_empty_input(self):
	"""
	scenario: using InteractiveOption with type=str and invoking without options
	behaviour: pressing enter on empty line at prompt repeats the prompt without a message
	"""
	cmd = self.simple_command(type=str)
	runner = CliRunner()
	result = runner.invoke(cmd, [], input='\nTEST\n')
	expected = 'Opt: \nOpt: TEST\nTEST\n'
	self.assertIsNone(result.exception)
	self.assertIn(expected, result.output)

[sphuber]

Good point. Looking into it more closely, I think the behavior is just the default prompting behavior of click. Take the following MWE:

@click.command()
@click.option('--label', required=False, type=str, prompt='Label')
def cmd(label):
    click.echo(f'Label: {label}')

This will keep prompting for the value each time an empty value is specified, even though it is not required. I think this is a fundamental behavior of click but I have the feeling this doesn't make any sense. If an option is not required and None would be perfectly fine, why would we keep prompting? I would say it makes a lot more sense to just accept that. The only downside I can see is if someone hits enter on accident, but that is the same as typing something incorrect and hitting enter, in that case you also cannot correct the "mistake" and simply have to reexecute the command. I really feel that this should be possible. What do you think? Do you think empty value for an optional prompted value should just be accepted?

[unkcpz]

I have no strong opinion on this. For a new user who may not understand the exact meaning of each opinion, I sometimes want to type ENTER to see what will happened. I kindly understand why click keep on repeat prompt. If we want do it in the way you mentioned, it would be better to add a instruction next to ? ! to let the user know that Enter will leave the option not set.

The other problem is, for example if we set is_on_computer=True, can we still set computer to None by hitting enter?

[codecov]

Codecov Report

Merging #5140 (b31e5f5) into develop (1dac2fe) will increase coverage by 0.18%.
The diff coverage is 90.15%.

❗ Current head b31e5f5 differs from pull request most recent head d212885. Consider uploading reports for the commit d212885 to get more accurate results

@@             Coverage Diff             @@
##           develop    #5140      +/-   ##
===========================================
+ Coverage    80.71%   80.89%   +0.18%     
===========================================
  Files          534      536       +2     
  Lines        36967    37069     +102     
===========================================
+ Hits         29835    29983     +148     
+ Misses        7132     7086      -46     

Flag	Coverage Δ
django	75.74% <80.29%> (+0.20%)	⬆️
sqlalchemy	74.84% <89.86%> (+0.20%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
aiida/backends/djsite/db/migrations/__init__.py	68.84% <0.00%> (ø)
...ns/12536798d4d3_trajectory_symbols_to_attribute.py	95.66% <ø> (ø)
...ns/ce56d84bcc35_delete_trajectory_symbols_array.py	82.15% <0.00%> (ø)
aiida/cmdline/commands/cmd_computer.py	81.29% <0.00%> (ø)
aiida/cmdline/commands/cmd_data/cmd_show.py	16.67% <0.00%> (ø)
aiida/cmdline/commands/cmd_profile.py	79.67% <ø> (ø)
aiida/cmdline/utils/decorators.py	70.18% <ø> (-1.01%)	⬇️
aiida/common/utils.py	76.89% <0.00%> (ø)
aiida/engine/__init__.py	100.00% <ø> (ø)
aiida/engine/processes/__init__.py	100.00% <ø> (ø)
... and 71 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1dac2fe...d212885. Read the comment docs.

[unkcpz]

@sphuber Let's close this for now. I made tests for prompt behavior with rebase during the coding week but indeed still got the issue from click side.