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

Rewrite command line parsing, add flags and expansions #12527

Merged
merged 1 commit into from
Feb 27, 2025

Conversation

the-mikedavis
Copy link
Member

@the-mikedavis the-mikedavis commented Jan 14, 2025

This change rewrites the command line parsing code (discarding shellwords) and adds command flags and expansions. Unfortunately these features are not really independent - each feature influences the design of the parsing and completion code - so I think it's best to add them all together.

The syntax and design are roughly the same as what Kakoune is doing but with a few twists:

  • %sh{..} expansions are recursive, enabling variables within shell commands like :noop %sh{gh browse "%{buffer_name}:%{cursor_line}" -c=%sh{git rev-parse HEAD}} for example from Command expansion v2 #11164 (comment). In contrast, Kakoune exposes $kak_<val_name> environment variables to the shell program.
  • A command's signature can be written to treat the input as quoted after a given number of arguments. This is used now in :toggle-option for example to support toggling objects and arrays like :toggle rulers [10, 20] [30, 40]: after the first arg everything is parsed as a JSON stream instead.

The documentation in book/src/command-line.md explains the quoting rules and how to use flags and expansions.

Completion in the command mode prompt supports expansion names, variable names, and flags and it can be extended in the future for new expansions.

This PR focuses on the parsing and completion changes so it only adds one flag and a handful of variables. Adding new flags, variables and expansions should be quite minimal. Here are some good example changes:

The user-facing changes are:

Review notes...

A good entrypoint looking at the new code is Args::parse. This is used when executing a command in helix_term::commands::typed::execute_command. Args reads Tokens from the Tokenizer, according to a typable command's declared Signature, expanding them when you hit <ret> in command mode. Signature tells Args how to parse the input: the range of positionals that should be expected, the command's flags and, optionally, after how many positionals the input should be consumed for further parsing within the command (Signature.raw_after, Tokenizer::rest).

The completion code in complete_command_args (typed.rs) is also worth a look to see how the Token type is reused outside of Args::parse.

Tokenizer closely follows Kakoune's CommandParser while Args somewhat follows Kakoune's ParametersParser.

shellwords has been discarded: the escaping rules were not very intuitive - generally backslash should be a convenience available to Unix and not at all required.

Fixes #5828
Fixes #10993
Closes #11164
Closes #12075
Closes #12624

@RoloEdits

This comment was marked as resolved.

@david-crespo
Copy link
Contributor

Exciting PR. These are working for me on macOS:

C-b = ":sh gh browse %{buffer_name}:%{cursor_line} -c=%sh{latest_pushed_commit}"
C-B = ":echo %sh{git blame --date=short -L %{cursor_line},+1 %{buffer_name} | sed -E 's/[0-9]+).*//' | sed 's/(//g'}"

@RoloEdits

This comment was marked as resolved.

@the-mikedavis

This comment was marked as resolved.

@RoloEdits

This comment was marked as resolved.

@the-mikedavis

This comment was marked as resolved.

@RoloEdits

This comment was marked as resolved.

@RoloEdits

This comment was marked as resolved.

@the-mikedavis

This comment was marked as resolved.

@the-mikedavis

This comment was marked as resolved.

@RoloEdits

This comment was marked as resolved.

@the-mikedavis

This comment was marked as resolved.

@RoloEdits

This comment was marked as resolved.

@the-mikedavis

This comment was marked as resolved.

@RoloEdits

This comment was marked as resolved.

Copy link
Contributor

@nik-rev nik-rev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor ideas here

@nik-rev
Copy link
Contributor

nik-rev commented Jan 14, 2025

When a variable is completed, we are left in this state:

:echo %{buffer_name

What would be nice is if it automatically inserted the}. This is a minor thing but It would slightly improve UX

@the-mikedavis
Copy link
Member Author

Ideally that would be covered by auto-pairs within the command line but that will take further changes to how completions work in the Prompt component

@tcoopman

This comment was marked as resolved.

@irh

This comment was marked as resolved.

@cgahr

This comment was marked as resolved.

@the-mikedavis

This comment was marked as resolved.

@cgahr

This comment was marked as resolved.

This is a full rewrite of the command line code which was parsed with
`Shellwords` in the past. `Shellwords`'s escaping system was quite
tricky to work with and prevented further improvements to the command
line syntax like command flags (e.g. `--reverse`) and Kakoune-like
expansions (`%sh{echo hello world}`).

Ideally these features would be added after improving the parsing code
but they both influence the design of the parser(s), their output types
and the completion code so I feel it's best to land them together in one
(admittedly outlandishly large) commit.

Co-authored-by: Pascal Kuthe <pascalkuthe@pm.me>
@the-mikedavis the-mikedavis merged commit 0efa820 into master Feb 27, 2025
6 checks passed
@the-mikedavis the-mikedavis deleted the command-line branch February 27, 2025 01:50
This was referenced Feb 27, 2025
@haras-unicorn
Copy link

thank you so much for this one! been waiting for quite a while for it :)

@rcorre
Copy link
Contributor

rcorre commented Feb 27, 2025

I'm not sure if this is a bug, or working as intended, but it seems that noop waits synchronously for the command to complete, whereas :sh runs a command without blocking the editor.

[keys.normal.g]
i = ":echo %sh{gh browse '%{buffer_name}:%{cursor_line}' -c}"
b = ":noop %sh{gh browse '%{buffer_name}:%{cursor_line}' -c}"
B = ":sh gh browse '%{buffer_name}:%{cursor_line}' -c"

Both gi and gb cause helix to briefly hang while the command executes. gB doesn't block the editor, but shows the output in a hover window. So I'm not sure :noop quite replaces the need for a :run-shell-command-no-output as-is.

However, :sh gh browse '%{buffer_name}:%{cursor_line}' -c &>/dev/null works fine to run a command without blocking the editor and without showing output. So maybe there's not a missing feature here, but I'm not sure when I'd use :noop.

P.S. thank you for this feature, it's great!

Dreaming-Codes added a commit to Dreaming-Codes/yazi-rs.github.io that referenced this pull request Feb 27, 2025
Due to this now merged PR helix-editor/helix#12527 % needs to be escaped with another % now
@david-crespo
Copy link
Contributor

@rcorre I'm confused about the output thing — gh browse doesn't have any output for me in helix — there is no popup even without the pipe to /dev/null. When I run it in the terminal, I see the Opening <url> in your browser message, and I confirmed it's going to stdout and not stderr. But I don't see it in helix when I run the command with :sh.

@rcorre
Copy link
Contributor

rcorre commented Feb 28, 2025

@david-crespo sorry for not clarifying -- the output is coming from my browser itself. I use qutebrowser, which prints "opening in existing instance". Though you're right, gh also has output and I'm not sure why that doesn't show up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment