Add --paging-full-mode option to commandline to determine if paging is complete

[thibaultmarin]

Description

Trying to prevent the TAB key from cycling through completions, I found this question
(https://unix.stackexchange.com/questions/565196/fish-shell-disable-pager-navigation-through-tab-shift-tab) offering a way to disable TAB when in pager mode (using bind \t 'if not commandline -P; commandline -f complete; end').

However, I would like to have the following functionality:

• first TAB press shows the reduced list of completion (limited to PAGER_UNDISCLOSED_MAX_ROWS)
• second TAB press shows all the completions (this is not the case with bind \t 'if not commandline -P; commandline -f complete; end')
• subsequent TAB presses do not cycle through completions.

I can achieve this by exposing the pager's full_disclosed flag and storing it in the commandline_state_t structure. I added an option to commandline (-F) to return this fully_disclosed flag and use the following for my TAB keybinding:

bind \t 'if not commandline -F; commandline -f complete; end'

which seems to achieve what I was looking for, without, as far as I can tell, breaking anything else.

Is there any interest in merging this change? Is my approach acceptable?

Thanks in advance for the guidance.

TODOs:

• Changes to fish usage are reflected in user documentation/manpages.
• Tests have been added for regressions fixed
• User-visible changes noted in CHANGELOG.rst

[krobelus]

I added an option to commandline (-F) to return this fully_disclosed flag and use the following for my TAB keybinding:
Is there any interest in merging this change? Is my approach acceptable?

I don't understand why you want to use a binding like that.
Maybe you got used to mashing Tab since bash is notoriously reluctant to show completion options? In fish that's not necessary, pressing Tab exactly twice will always go into full paging mode if there are enough completions.

That said, it seems fine to add this.

[krobelus]

this would be more consistent with others

Suggested change

	bool get_fully_disclosed() const;
	bool is_fully_disclosed() const;

also the comment is off now, can you move the new method in a separate paragraph like the other methods above?

[krobelus]

you forgot to update the check for incompatible options
at src/builtins/commandline.cpp:332

[krobelus]

I considered giving the existing -P/--paging-mode an optional argument, like --paging-mode=full, but that would work slightly worse with interactive completions, so yours seems fine.

Maybe just remove the short option, since it's not so obvious? It's okay to keep it if you like.

[ridiculousfish]

LGTM, please fix the issues krobelus identified and I'll merge. Thank you.

[thibaultmarin]

Thanks for the feedback, it helped me realize that my initial implementation was not exactly what I wanted.

I would like to avoid the slightly inconsistent behavior of the TAB key depending on the length of the completion list: short (fits within PAGER_UNDISCLOSED_MAX_ROWS) or long. A double press on TAB will start cycling through completions when the list is short and show a full (filling the screen) list of completions when the list is long (a third TAB press would start cycling through completions). Since I don't mind losing the ability to cycle through possibly long lists of completions using TAB, I would rather have TAB stop interacting with the completion list once it is fully shown (in other words, I would like to disable TAB if the next TAB press will start cycling through completions).

The previous version worked only for long lists. This version relies on the remaining_to_disclose variable, which seems to work regardless of the size of the completion list.

Combined with the following binding:

bind \t 'if not commandline --paging-full-mode; commandline -f complete; end'

I get the desired behavior.

Please let me know if this is an acceptable approach.

Thanks again for the guidance.

[krobelus]

That all makes sense, merged with some minor changes in ceade16