Command expansion v2

[tdaron]

Hi !

We talked about command expansions inside of #6979, but I created a new PR as the original PR reached a "stopping" point as the PR became inactive, mainly because the OP is currently busy.

Into this new PR , i rebased #6979 on master, and made some changes as @ksdrar told me to ;) (I think whitespaces are fixed as i handle completions inside of the shellwords parsing phase)

If you think creating a new PR is not a good idea i can of course close this one, but I think it would be great to finally get this feature !

Current variables:

Variable	Description
%{basename} or %{b}	The name and extension of the currently focused file.
%{dirname} or %{d}	The absolute path of the parent directory of the currently focused file.
%{cwd}	The absolute path of the current working directory of Helix.
%{git_repo}	The absolute path of the git repository helix is opened in. Fallback to cwd if not inside a git repository
%{filename} or %{f}	The absolute path of the currently focused file.
%{filename:rel}	The relative path of the file according to cwd (will give absolute path if the file is not a child of the current working directory)
%{filename:git_rel}	The relative path of the file according to git_repo (will give absolute path if the file is not a child of the git directory or the cwd)
%{ext}	The extension of the current file
%{lang}	The language of the current file
%{linenumber}	The line number where the primary cursor is positioned.
%{cursorcolumn}	The position of the primary cursor inside the current line.
%{selection}	The text selected by the primary cursor.
%sh{cmd}	Executes cmd with the default shell and returns the command output, if any.

[tdaron]

I think this is good for review (lints should be good now)

[RoloEdits]

Rather than tying this to Shellwords parsing, it makes more sense to me if it were to function as an ultra fancy str::replace that would return a Cow<str> that would then be passed to Shellwords, with Shellwords::from(&args), with it none the wiser that anything happened.

Just like str::replace, it would copy to a new buffer and then replace when comes across the pattern(variable).

Perform all the replacements for the variables first and then evaluate the %sh if it needs to. Starting from inner %sh first and using the return from that in the parent %sh call.

I am working on #11149 to try to simplify the handling of args, and the way this is currently implemented would require a lot of changes for whichever would get merged second. Mostly with this pr's logic needing a complete rewrite.

[tdaron]

Yup but isn't shellword parsing done at each keypress for completion,... ?

Would replacing before parsing using shellwords mean executing %sh at each keypress ? (I mean, each key typed after the %sh{...}

[RoloEdits]

Yeah, currently this pr touches shellwords. Which I don't think it should.

Unless im missing something, this can be done with interpreting the args as the text input, %{filename}, and then when the command is actually ran it can expand the variables.

[tdaron]

It worked like this in the original PR but shellwords messed up with spaces inside variables expansion (eg: %sh{there are some spaces}) so I added a lil exception inside shellword to ignore spaces (actually, ignore everything) inside of a %{} but the expansion is only done when the command is ran

[RoloEdits]

Ah, I see. With the changes I am making for the args I changed the MappableCommand::Typeable.args to a String

pub enum MappableCommand {
    Typable {
        name: String,
        args: String,
        doc: String,
    },
    Static {
        name: &'static str,
        fun: fn(cx: &mut Context),
        doc: &'static str,
    },
}

Instead of the current master Vec<String>. In this way I can just make an iterator over the string to parse out the proper arguments lazily.

    pub fn execute(&self, cx: &mut Context) {
        match &self {
            Self::Typable { name, args, doc: _ } => {
                if let Some(command) = typed::TYPABLE_COMMAND_MAP.get(name.as_str()) {
                    let mut cx = compositor::Context {
                        editor: cx.editor,
                        jobs: cx.jobs,
                        scroll: None,
                    };

                    if let Err(err) =
                        (command.fun)(&mut cx, Args::from(args), PromptEvent::Validate)
                    {
                        cx.editor.set_error(format!("{err}"));
                    }
                }
            }
            Self::Static { fun, .. } => (fun)(cx),
        }
    }

If we were to treat the expanding as a fancy replace, we can just replace the String to make a Cow<str>, in case it doesnt have any variables to expand, and then pass that to Args::from(&args)

This could then be some rough form of the replacing:

fn expand_variables<'a>(editor: &Editor, args: &'a str) -> anyhow::Result<Cow<'a, str>> {
    let (view, doc) = current_ref!(editor);

    let mut expanded = String::with_capacity(args.len());
    let mut var = Tendril::new_const();
    let mut chars = args.chars().peekable();

    while let Some(c) = chars.next() {
        if c == '%' {
            if let Some('{') = chars.peek() {
                chars.next(); // consume '{'

                while let Some(&ch) = chars.peek() {
                    if ch == '}' {
                        chars.next(); // consume '}'
                        break;
                    }
                    var.push(ch);
                    chars.next();
                }

                match var.as_str() {
                    "basename" => {
                        let replacement = doc
                            .path()
                            .and_then(|it| it.file_name().and_then(|it| it.to_str()))
                            .unwrap();

                        expanded.push_str(replacement);
                    }
                    "filename" => {
                        let replacement = doc
                            .path()
                            .and_then(|path| path.parent())
                            .unwrap()
                            .to_str()
                            .unwrap();
                        expanded.push_str(replacement);
                    }
                    "dirname" => {
                        let replacement = doc
                            .path()
                            .and_then(|p| p.parent())
                            .and_then(std::path::Path::to_str)
                            .unwrap();
                        expanded.push_str(replacement);
                    }
                    "cwd" => {
                        let dir = helix_stdx::env::current_working_dir();
                        let replacement = dir.to_str().unwrap();
                        expanded.push_str(replacement);
                    }
                    "linenumber" => {
                        let replacement = (doc
                            .selection(view.id)
                            .primary()
                            .cursor_line(doc.text().slice(..))
                            + 1)
                        .to_string();

                        expanded.push_str(&replacement);
                    }
                    "selection" => {
                        let replacement = doc
                            .selection(view.id)
                            .primary()
                            .fragment(doc.text().slice(..));

                        expanded.push_str(&replacement);
                    }
                    unknown => bail!("unknown variable `{unknown}`"),
                }

                // Clear for potential further variables to expand.
                var.clear();
            } else {
                expanded.push(c);
            }
        } else {
            expanded.push(c);
        }
    }

    //... `%sh` stuff

    Ok(expanded.into())
}

to use it like :

match expand_variables(cx.editor, args) {
    Ok(args) => {
        if let Err(err) =
            (command.fun)(&mut cx, Args::from(&args), PromptEvent::Validate)
        {
            cx.editor.set_error(format!("{err}"));
        }
    }
    Err(err) => cx.editor.set_error(format!("{err}")),
};

[RoloEdits]

A working concept with what I posted:

"A-," = [":sh echo `%{basename} %{filename} %{dirname} %{cwd} %{linenumber} %{selection}`"]

Unless there is some other reason for parsing the variables in Shellwords, I think this separates things pretty well.

[tdaron]

Make sense, it's much clearer this way to me

Shall I update this PR to be rebased on yours or shall we directly integrate command expansions inside your refactoring ?

[RoloEdits]

I'm not sure what changes will be proposed for my Args refactor pr, or if it might just be rejected altogether. For trying to coordinate multiple prs, I think we should ask for some help from the core maintainers. Perhaps @archseer @the-mikedavis or @pascalkuthe has some direction they would like us to follow?

[tdaron]

Unless there is some other reason for parsing the variables in Shellwords, I think this separates things pretty well.

Don't completions need this ?

[RoloEdits]

You mean for the :echo command introduced here? I believe this could be a static thing with the CommandSigniture.

[tdaron]

Nope, I am talking about the code inside the command_mode function (file typed.rs) to provide completion (but as I removed all completion specific code from the original PR I might have misunderstood how it works)

(If the user type %{filen the editor shall provide %{filename} completion)

And if shellwords is messing up spaces inside of expansions I think it can mess up the completion part. But there is no reason a variable name would contain space like %{space here}. The only exception is inside some complex ones like %sh so I don't know if handling them worth it

[RoloEdits]

Yeah, I believe you can provide a Vec<&str> , vec!["%{filename}"], that gets resolved by the completer. When a space is the final part of an input, for example, it should present the options given in the CommandSigniture. The issue is that with %sh{}, if you want to put variable inside it, I don't think the completer will work, as the last part of the input would be }.

        |editor: &Editor, input: &str| {
            let shellwords = Shellwords::from(input);
            let command = shellwords.command();

            if command.is_empty()
                || (shellwords.args().next().is_none() && !shellwords.ends_with_whitespace())
            {
                fuzzy_match(
                    input,
                    TYPABLE_COMMAND_LIST.iter().map(|command| command.name),
                    false,
                )
                .into_iter()
                .map(|(name, _)| (0.., name.into()))
                .collect()
            } else {
                // Otherwise, use the command's completer and the last shellword
                // as completion input.
                let (word, len) = shellwords
                    .args()
                    .last()
                    .map_or(("", 0), |last| (last, last.len()));

                TYPABLE_COMMAND_MAP
                    .get(command)
                    .map(|tc| tc.completer_for_argument_number(argument_number_of(&shellwords)))
                    .map_or_else(Vec::new, |completer| {
                        completer(editor, word)
                            .into_iter()
                            .map(|(range, file)| {
                                let file = shellwords::escape(file);

                                // offset ranges to input
                                let offset = input.len() - len;
                                let range = (range.start + offset)..;
                                (range, file)
                            })
                            .collect()
                    })
            }
        }, // completion

For instance this is how :theme and :set work, though those are not just a vec![] as they reference other aspects of how helix works. This would only be for a specific command though, like :echo, and wouldn't be a blanket way to get completions. But I also don't think that is wanted either.

[RoloEdits]

And perhaps its actually not desired to parse in a way that respects whitespace? With the Args here being an iterator, you could filter all that aren't } and then last that? But when entering the arg you would have to make sure that its not like %sh{echo hello} as the last } would be touching, and without special handling, would be part of last's output.

If you wrote like %sh{echo 'hello'} it would be fine as the part in the quotes would be one arg, so } would be filtered out.

                let (word, len) = shellwords
                    .args()
                    // Special case for `%sh{...}` completions so that final `}` is excluded from matches.
                    // NOTE: User would have to be aware of how to enter input so that final `}` is not touching
                    // anything else.
                    .filter(|arg| *arg != "}")
                    .last()
                    .map_or(("", 0), |last| (last, last.len()));

[daedroza]

Is there a way we can increase the pop-up size? I'm attempting to get git log in a large pop-up instead of small one.

[tdaron]

Is there a way we can increase the pop-up size? I'm attempting to get git log in a large pop-up instead of small one.

Wouldn't more advanced solutions better suit your usecase ? (like a floating pane in zellij containing it (you can open it from helix) or something like that ?)

But it should be possible, but not easy as this PR do not touch the TUI code part. We are using code already written and well integrated into helix. I don't know how hard it can be, and I think it's quite out of the scope of this PR.

[danillos]

I did a test with :echo %sh{git blame -L %{linenumber} %{filename}}

and it is returning;

%sh{git blame -L 14 file.rb} 

So it didn't evaluate the shell script.

[tdaron]

Hi !

Just added those :) You now have filename:git_rel as well as filename:rel ! 🎉

Also added git_repo.

[david-crespo]

Thank you, that's fantastic. I can now do in one line what I was previously doing with a whole bash function.

C-b = ":sh gh browse %{filename:git_rel}:%{linenumber} -c=%sh{git rev-parse HEAD}"

(If the current commit hasn't been pushed yet this will 404, but that was a problem with my current function anyway.)

[hakan-demirli]

@tdaron There is a regression on cursorcolumn feature.

Helix: 237cbe4
PR: 385ea7c
Command: sh echo browse %{filename}:%{linenumber}:%{cursorcolumn}

Cursor Column keeps increasing after each row:

It is only correct for the first row.

[7flash]

Thank you, that's fantastic. I can now do in one line what I was previously doing with a whole bash function.

C-b = ":sh gh browse %{filename:git_rel}:%{linenumber} -c=%sh{git rev-parse HEAD}"

(If the current commit hasn't been pushed yet this will 404, but that was a problem with my current function anyway.)

How can I do this?

filename="$2"

^ do I need to recompile helix at this PR to have access to filename like in your bash script?

[david-crespo]

@7flash Yes, you need to build from source. You could clone the fork directly and check out this branch, or (what I do) clone the main repo and add git add remote this fork as a remote. That way I can merge several PRs into my custom branch.

[tdaron]

@hakan-demirli cursor column should be fixed :)

[carterols]

Man, I'm excited to use this! Thanks for working on it. Hopefully it can get reviewed/merged soon.

[tdaron]

Me too !
Just waiting for #11149 to be merged to rebase my work on it :)

[baldwindavid]

@carterols - I've seen nothing to suggest it won't be merged eventually, but this PR has its roots in another PR from back in early last year so I'd suggest building from this branch or merging into your own if needing it. I've happily been doing so and it provides all the interoperability for the cli tooling I need (running tests under cursor, blame, open in Github, git explorer, file explorer, interactive find and replace, etc). I know there's been a lot of discussion around an extension system, but I'm unsure whether I will actually want extensions anyway as opposed to vanilla Helix + best-of-breed cli tools. Right now we have neither in core, but pulling the branch and installing is easy enough for the only critical need I have.

[ataractic]

This feature really is a must have! I don't understand why it is not in the next release.

[tdaron]

Just merged master into this branch. Everything should be working fine, let me know if something broke

[7flash]

Please merge, it will allow to execute all kinds of scripts, for exampl formatting currently opened buffer, assigned to Ctlr+F hotkey.

[tdaron]

Please merge, it will allow to execute all kinds of scripts, for exampl formatting currently opened buffer, assigned to Ctlr+F hotkey.

This PR cannot be merged until #11149 is done.
However, for your formating hotkey you can do it without it by selecting all the buffer, piping it into your formatter and replacing it with the result. (You can also directly use :format from helix which is simpler)

[7flash]

Please merge, it will allow to execute all kinds of scripts, for exampl formatting currently opened buffer, assigned to Ctlr+F hotkey.

This PR cannot be merged until #11149 is done. However, for your formating hotkey you can do it without it by selecting all the buffer, piping it into your formatter and replacing it with the result. (You can also directly use :format from helix which is simpler)

Currently, I have hotkey in my helix config, however it applies to all files in the directory:

[keys.normal."+"]
f = ":run-shell-command deno fmt"

Trying to achieve a desired behavior, which does formatting only current file, I followed your suggestion. I've selected file into a buffer with Shift-% then pressed "|" to pipe a "deno fmt" command, but unfortunately it replaced my buffer with empty string.

I understand it might be because "deno fmt" does not expect a buffer but allows to pass a file path. In this case, I could write a program in Bun which expects a buffer and then passing it through formatter API as a temporary file, however the issue here that I would have to type full program path every time, to pipe through it, because it does not recognize my functions defined in ~/.zshrc.

[darshanCommits]

Why not use the built-in formatter command.
It works on stdin, deno fmt also supports it.

Just use the command deno fmt - add to your configuration and it'll work seamlessly.

[objectiveryan]

Please merge, it will allow to execute all kinds of scripts, for exampl formatting currently opened buffer, assigned to Ctlr+F hotkey.

This PR cannot be merged until #11149 is done. However, for your formating hotkey you can do it without it by selecting all the buffer, piping it into your formatter and replacing it with the result. (You can also directly use :format from helix which is simpler)

With #4709 (which has already been merged into master but isn't yet in an official release) you can make key bindings like "@:sh sometool <C-r>%<ret>" to run sometool on the current file.

[nestor-custodio]

Friendly reminder that some tooling behaves differently depending on which file you're running it against (sometimes based on the path, sometimes on the basename, sometimes on the extension, etc), and comments to the effect of "you can just pipe your file to whatever tool you want" or "just use the built-in :format/:fmt" make it seem like this PR is a nice-to-have when it in fact addresses a need many of us are currently having to work around.

To be clear, I'm not saying "don't help folks whose needs can be met with something like the built-in LSP formatter or a custom keybind/command" -- I'm saying please keep in mind any such suggestion may have effects beyond what the requester intended (depending on the tooling in question), and that in some cases there may not be any way to do (via current means) what this PR will allow.

Again: just a reminder to those helping and a heads-up to those asking for help.

[NikitaRevenco]

Would be nice if we have some shorthands for common expansions

• %{f} same as %{filename}
• %{d} same as %{dirname}
• %{b} same as %{basename}

[tdaron]

I do not think this is necessary, as those expansions will be almost only used inside a config file I think it's not a problem to have to write full variables names (and if they are used in the "interactive" command prompt a completion prompt could be created to be fast). Shorthands would increase code complexity as well as documentation for a (too) small QOL improvement imo

[NikitaRevenco]

I do not think this is necessary, as those expansions will be almost only used inside a config file I think it's not a problem to have to write full variables names (and if they are used in the "interactive" command prompt a completion prompt could be created to be fast). Shorthands would increase code complexity as well as documentation for a (too) small QOL improvement imo

I merged your PR into my personal fork as well as #11285 and I've been using it for a few days. I gotta say, I'm seriously impressed by it

These two PRs together are really going to be game-changing. In my opinion, the only thing helix really needs is a file manager, above all else. #11285 is great at providing the functionality to browse the file tree and open files, while your PR is extremely useful for creating, renaming, deleting and copying files.

For example, I frequently run the following commands:

• Change directory to current file's directory: :cd %{dirname}
• Create a new file in the current directory: :o %{dirname}/file.txt
• Delete current file :sh rm %{filename}
• Copy file to parent directory :sh cp %{filename} %{dirname}/..
• Create a new file in sibling directory: :o %{dirname}/../lib/file.txt

Helix doesn't even need a "full-blown" file manager because 99% of the functionality you would want will be available once this PR and #11285 gets merged.

That's initially why I suggested to add shorthands for common variables. at least for me, I don't use it just in config files, but actively use the feature to navigate across the file system, completely erasing the need for anything more complex. Saving several keystrokes so often would be a nice QoL.

Regarding complexity of the change, I took a look at it and it turned out to be fairly straightforward. In helix-view/src/editor/variable_expansion.rs making the following changes on line 44, 51 and 77 is all that it takes to add the aliases:

- "basename" => doc
+ "basename" | "b" => doc

- "filename" => doc
+ "filename" | "f" => doc

- "dirname" => doc
+ "dirname" | "d" => doc

Introducing a completion is a great idea if you decide not to add this anyway, but its implementation would likely add a lot more complexity than adding aliases.

Even if a completion prompt was added, it would still be a few extra keystrokes for common operations (e.g. to tab through the list and then hit enter to select the one you want), so that's why I advocate for adding these. It's completely up to you though!

Having said that, I'm definitely looking forward to these two PRs getting into one of the next releases, as I think a lot of people who previously wouldn't use Helix will consider it more than before. Thank you!

[tdaron]

Hey !
Thanks for this full explanation, I can now see the need for those shorthands, even if I think that having a completion system for variables expansions might solve this issue, allowing to write %{dirname} by only writing %{di then enter. But as this system is not implemented (I don't know if it's better to put it in another PR or in this one, and in both cases it will be a lot easier once #11149 got merged) I will add those shorthands :)

(When talking about code complexity I didn't meant implementation difficulty but rather a "keep it simple" approach to avoid adding unnecessary things)

(And completion would be a much more complex implementation but it would allow discovery of those variables without having to read the documentation everytime)

[tcoopman]

Thanks, loving this PR, now this is a lot easier: ":sh tmux send-keys -t '{right}' 'mix test %{filename:rel}:%{linenumber}' ENTER"

One question/remark - given that there are some PRs to integrate jujutsu as a vcs, I'm wondering if it doesn't make more sense to rename git_repo and git_rel to just repo and repo_rel?

[tdaron]

As a jujutsu user I couldn't agree more with you :)