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.
%{repo}	The absolute path of the VCS repository helix is opened in. Fallback to cwd if not inside a VCS 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:repo_rel}	The relative path of the file according to repo (will give absolute path if the file is not a child of the VCS 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]

Yep, atm the PR does not handle completions inside of %sh, but I am waiting an answer from core maintainers about which direction we should follow with @RoloEdits as I don't want to waste my time writing code on this if it's going to be removed and re-written anyway haha

[tdaron]

But you should get the desired behavior using :sh git blame -L %{linenumber} %{filename} I think, I didn't test

[danillos]

But you should get the desired behavior using :sh git blame -L %{linenumber} %{filename} I think, I didn't test

This was just an example. What I need is:

s = ':open %sh{~/.dotfiles/support/helix_command.rb switch_to_spec %{filename}}'

I moved back to the previous PR. It was working there.

[daedroza]

Yep, atm the PR does not handle completions inside of %sh, but I am waiting an answer from core maintainers about which direction we should follow with @RoloEdits as I don't want to waste my time writing code on this if it's going to be removed and re-written anyway haha

I don't think the core maintainers will allow this patch. Such kind of extensions will be covered with the Scheme language paradigm. I'd a similar issue where I wanted the current document's indentation size passed to formatter.

Most likely when #10389 is fully ready probably.

Though I wish patches like yours are merged along with #11149

[sanfilippopablo]

As for some feedback on names, I wonder how people would feel about these changes?:

basename -> file
dirname -> dir
filename -> path
linenumber -> line
cursorcolumn -> column

I like those changes

[nik-rev]

As for some feedback on names, I wonder how people would feel about these changes?:

basename -> file
dirname -> dir
filename -> path
linenumber -> line
cursorcolumn -> column

I like those changes

I think these are more descriptive:

basename -> filename
dirname -> current-file-directory
filename -> current-file-path
linenumber -> line
cursorcolumn -> column

Reasons:

• filename is less ambiguous than file. For example, I might think "file? Maybe that's the path of the current file"
• dir can be confused for current working directory. current-file-directory is more descriptive
• path could also be confused for other things. for example, path of the current directory. ENV $PATH. current-file-path

current-file-directory and current-file-path are long, but we have shorthands so it's fine. Similar to how there is :change-current-directory. No one probably uses the full version, but if we have shorthands we may as well use longer, more descriptive names

[nik-rev]

But in all honesty, discussions over these details may actually delay this PR so it won't be in the next release. What about if we merge it as-is, and then make further enhancements down the line if necessary?

[tdaron]

No matter what, this PR won't be included in the next release, as it depends on #11149, which will be merged after the next release.

I really like line and column. However I agree that file isn't descriptive enough, as well as dir (but dirname isn't that nice either).

I do think that:

filename
current_dir
path
line
column

Is great. But yes I think everyone will have a different opinion and those names could even change after this PR so it's not that important

[RoloEdits]

Yes, if you look at https://github.com/helix-editor/helix/blob/changelog/CHANGELOG.md it says the first of the year. As not even one maintainer has left any reviews here yet, it wont be included for sure. Any big features will have to wait till next release cycle.

And also, naming is hard. To me these can be succinct as the context is already set. These aren't from an unknown scope. Its all around the current buffer.

(current buffer) file
(current buffer) path
(current buffer) column
(current buffer) line
(current buffer) dir

etc.

Im sure this can change later on, but as far as I know, only one other variable is being worked on that is not in some way tied to the current buffer(with them being either a leaf or a stem to the buffer), that being %{arg}, for use with custom typable commands.

[sanfilippopablo]

I don't think the names should be completely self-descriptive. After all, there's documentation with the full meaning if one's unsure about the meaning of any. And conciseness has value, although I'm not sure how much these substitutions will be used "live" instead of living in a keybinding/command in the config file.

[RoloEdits]

#11149 was merged into master, so this can now be rebased off of master. Should be pretty straight forward as the changes were already made.

Mainly, for commands.rs, in the yanked_joined_impl:

- acc.push_str(&helix_core::shellwords::unescape(separator));
+ acc.push_str(&shellwords::unescape(separator));

And for commands::typed.rs, in the completer_for_argument_number:

- .map_or(&self.signature.var_args, |completer| completer)
+ .unwrap_or(&self.signature.var_args)

[tdaron]

Hey !

I was trying to work on this but I noticed #11149 got unmerged from master inside of 2178186

[RoloEdits]

Ah, yes, I forgot to say something here: the maintainers felt more and more like they wanted a different API. The new work is being tracked here #12441.

Most of what the old work had done was liked, which is the bulk of the work, just wanted some more rules around escaping and more eager approach rather than a lazy one. Hopefully this can get merged soon. Sorry for the trouble on this.

[tdaron]

No problem :)

[RoloEdits]

Lucky I think the way that this PR is now setup it shouldn't be bad to merge. I will provide suggestions for conflict fixes when the time comes.

[cor]

For those already using this PR, I wrote a small utility gh-permalink that copies the github permalink of your current line.

[sanfilippopablo]

No way! I wrote the same thing a couple days ago, seems like we all are wanting this PR for similar reasons:

#!/usr/bin/env fish

function get_git_provider_url --argument filename line_number
    if not git rev-parse --is-inside-work-tree >/dev/null 2>&1
        echo "Error: Not inside a git repository."
        return 1
    end

    set remote_url (git config --get remote.origin.url)
    if test -z "$remote_url"
        echo "Error: No remote origin found."
        return 1
    end

    if string match -q '*github.com*' $remote_url
        set provider github
    else if string match -q '*gitlab.com*' $remote_url
        set provider gitlab
    else
        echo "Error: Unsupported git provider."
        return 1
    end

    set repo_path (echo $remote_url | perl -nE 'say $2 if /\.com(:|\/)(.*)\.git/')
    set branch (git rev-parse --abbrev-ref HEAD)

    switch $provider
        case github
            echo "https://github.com/$repo_path/blob/$branch/$filename#L$line_number"
        case gitlab
            echo "https://gitlab.com/$repo_path/-/blob/$branch/$filename#L$line_number"
    end
end

if [ (count $argv) -ne 2 ]
    echo "Usage: get_git_url.fish <filename> <line_number>"
    return 1
end

get_git_provider_url $argv[1] $argv[2]