Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: readline quoting breaks "keyname: function-name or macro" style #3611

Closed
2 tasks done
n8henrie opened this issue Jan 21, 2023 · 5 comments · Fixed by #3947
Closed
2 tasks done

bug: readline quoting breaks "keyname: function-name or macro" style #3611

n8henrie opened this issue Jan 21, 2023 · 5 comments · Fixed by #3947
Assignees
Labels
bug status: stale triage Issues or feature request that have not been triaged yet

Comments

@n8henrie
Copy link
Contributor

Are you following the right branch?

  • My Nixpkgs and Home Manager versions are in sync

Is there an existing issue for this?

  • I have searched the existing issues

Issue description

https://www.gnu.org/software/bash/manual/html_node/Readline-Init-File-Syntax.html

The (IMO) much more readable alternative to the "keyseq" style as used in the example:

{ "\\C-h" = "backward-kill-word"; }

would be: { Control-h = "backward-kill-word"; }

Unfortunately the current implementation quotes Control-h and outputs "Control-h", which doesn't work. Obviously one could still put it in extraConfig, but it would be nice if it didn't default to adding quotes that I didn't ask for.

I think it makes sense to expect users to explicitly escape quotes if they want them, since it seems equally valid and more readable to not have them in many cases.

mkBindingStr = k: v: ''"${k}": ${v}'';

It seems like the most straightforward backward-compatible way to go about this would be adding a new option (keynames? unquotedBindings?) happy to make a PR if it seems reasonable.

Maintainer CC

@vojta001
@rycee

System information

- system: `"aarch64-darwin"`
 - host os: `Darwin 22.2.0, macOS 13.1`
 - multi-user?: `yes`
 - sandbox: `no`
 - version: `nix-env (Nix) 2.11.1`
 - channels(n8henrie): `"darwin, nixpkgs-22.11"`
 - channels(root): `""`
 - nixpkgs: `/Users/n8henrie/.nix-defexpr/channels/nixpkgs`
@n8henrie n8henrie added bug triage Issues or feature request that have not been triaged yet labels Jan 21, 2023
@ncfavier
Copy link
Member

ncfavier commented Jan 21, 2023

A simple heuristic would be to consider that a key is a keyseq if it contains a backslash, and a keyname otherwise. I'm pretty confident that keynames can't contain backslashes, less confident about the other way around unfortunately. Does it make sense at all to define a binding for a keyseq without any modifiers? EDIT: at least it seems to work...

If this can't be done I'd go with adding a new option keynameBindings.

@n8henrie
Copy link
Contributor Author

n8henrie commented Jan 21, 2023

Don't think that will always work. This is a valid keyseq:

"ab": "asdf"

EDIT: a then b work normally, holding a then hitting b inserts asdf.

@stale
Copy link

stale bot commented Apr 21, 2023

Thank you for your contribution! I marked this issue as stale due to inactivity. Please be considerate of people watching this issue and receiving notifications before commenting 'I have this issue too'. We welcome additional information that will help resolve this issue. Please read the relevant sections below before commenting.

If you are the original author of the issue

  • If this is resolved, please consider closing it so that the maintainers know not to focus on this.
  • If this might still be an issue, but you are not interested in promoting its resolution, please consider closing it while encouraging others to take over and reopen an issue if they care enough.
  • If you know how to solve the issue, please consider submitting a Pull Request that addresses this issue.

If you are not the original author of the issue

  • If you are also experiencing this issue, please add details of your situation to help with the debugging process.
  • If you know how to solve the issue, please consider submitting a Pull Request that addresses this issue.

Memorandum on closing issues

Don't be afraid to manually close an issue, even if it holds valuable information. Closed issues stay in the system for people to search, read, cross-reference, or even reopen – nothing is lost! Closing obsolete issues is an important way to help maintainers focus their time and effort.

@stale stale bot added the status: stale label Apr 21, 2023
n8henrie added a commit to n8henrie/home-manager that referenced this issue May 4, 2023
n8henrie added a commit to n8henrie/home-manager that referenced this issue May 4, 2023
Kenames like `Control-n` or `meta-p` should not be quoted or they don't work.

Keyseqs like `\C-p` or `ab` must continue to be quoted.

See also: https://www.gnu.org/software/bash/manual/html_node/Readline-Init-File-Syntax.html

Fixes nix-community#3611
n8henrie added a commit to n8henrie/home-manager that referenced this issue May 4, 2023
Kenames like `Control-n` or `meta-p` should not be quoted or they don't work.

Keyseqs like `\C-p` or `ab` must continue to be quoted.

See also: https://www.gnu.org/software/bash/manual/html_node/Readline-Init-File-Syntax.html

Fixes nix-community#3611
n8henrie added a commit to n8henrie/home-manager that referenced this issue May 4, 2023
Kenames like `Control-n` or `meta-p` should not be quoted or they don't work.

Keyseqs like `\C-p` or `ab` must continue to be quoted.

See also: https://www.gnu.org/software/bash/manual/html_node/Readline-Init-File-Syntax.html

Fixes nix-community#3611
n8henrie added a commit to n8henrie/home-manager that referenced this issue May 4, 2023
Kenames like `Control-n` or `meta-p` should not be quoted or they don't work.

Keyseqs like `\C-p` or `ab` must continue to be quoted.

See also: https://www.gnu.org/software/bash/manual/html_node/Readline-Init-File-Syntax.html

Fixes nix-community#3611
ncfavier pushed a commit that referenced this issue May 4, 2023
Kenames like `Control-n` or `meta-p` should not be quoted or they don't work.

Keyseqs like `\C-p` or `ab` must continue to be quoted.

See also: https://www.gnu.org/software/bash/manual/html_node/Readline-Init-File-Syntax.html

Fixes #3611
antholeole pushed a commit to antholeole/home-manager that referenced this issue May 21, 2023
Kenames like `Control-n` or `meta-p` should not be quoted or they don't work.

Keyseqs like `\C-p` or `ab` must continue to be quoted.

See also: https://www.gnu.org/software/bash/manual/html_node/Readline-Init-File-Syntax.html

Fixes nix-community#3611
sysedwinistrator pushed a commit to sysedwinistrator/home-manager that referenced this issue May 30, 2023
Kenames like `Control-n` or `meta-p` should not be quoted or they don't work.

Keyseqs like `\C-p` or `ab` must continue to be quoted.

See also: https://www.gnu.org/software/bash/manual/html_node/Readline-Init-File-Syntax.html

Fixes nix-community#3611
@hpfr
Copy link
Contributor

hpfr commented Sep 19, 2023

#3947 does not fully resolve this. From the Readline manual:

A number of symbolic character names are recognized while processing this key binding syntax: DEL, ESC, ESCAPE, LFD, NEWLINE, RET, RETURN, RUBOUT, SPACE, SPC, and TAB.

The implementation in #3947 treats these character names as key sequences. For example, "TAB" = "menu-complete" binds menu-complete to the literal sequence T A B.

Just adding those names to the list at

builtins.elem (builtins.head (lib.splitString "-" (toLower k))) [
"control"
"meta"
];
would probably fix it. Otherwise, it might just be best to have separate attributes for key names and key sequences.

@n8henrie
Copy link
Contributor Author

True! I made some of these points in the discussion in the PR, and read (and believe i linked to) the manual as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug status: stale triage Issues or feature request that have not been triaged yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants