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.

Sign up for GitHub

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

command expansions #3393

Closed
wants to merge 5 commits into from
Closed

command expansions #3393

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6f6cb3c
support commmand expansion
QiBaobin Nov 21, 2022
784380f
Merge remote-tracking branch 'origin/master'
QiBaobin Jan 24, 2023
85d38a7
remove command group function
QiBaobin Jan 24, 2023
22d17f9
Update helix-term/src/commands/typed.rs
QiBaobin Jan 26, 2023
7e7c0dc
Merge branch 'master' into cmd-expansions
QiBaobin Apr 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions book/src/generated/typable-cmd.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,4 @@
| `:append-output` | Run shell command, appending output after each selection. |
| `:pipe` | Pipe each selection to the shell command. |
| `:run-shell-command`, `:sh` | Run a shell command |
| `:commands`, `:cmds` | Run commands together, use && to sepearte them |
QiBaobin marked this conversation as resolved.
Show resolved Hide resolved
21 changes: 11 additions & 10 deletions helix-term/src/commands.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,16 +165,17 @@ impl MappableCommand {
pub fn execute(&self, cx: &mut Context) {
match &self {
Self::Typable { name, args, doc: _ } => {
let args: Vec<Cow<str>> = args.iter().map(Cow::from).collect();
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(e) = (command.fun)(&mut cx, &args[..], PromptEvent::Validate) {
cx.editor.set_error(format!("{}", e));
}
let mut cx = compositor::Context {
editor: cx.editor,
jobs: cx.jobs,
scroll: None,
};
if let Err(e) = typed::process_cmd(
&mut cx,
&format!("{} {}", name, args.join(" ")),
PromptEvent::Validate,
) {
cx.editor.set_error(format!("{}", e));
}
}
Self::Static { fun, .. } => (fun)(cx),
Expand Down
149 changes: 124 additions & 25 deletions helix-term/src/commands/typed.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1739,6 +1739,67 @@ fn run_shell_command(
Ok(())
}

fn cmds(cx: &mut compositor::Context, args: &[Cow<str>], event: PromptEvent) -> anyhow::Result<()> {
if event != PromptEvent::Validate {
return Ok(());
}

let mut start;
let mut end = 0;
loop {
start = if end == 0 { 0 } else { end + 1 };
end = start + 1;
while end < args.len() {
if args[end] == "&&" {
break;
}
end += 1;
}

if start >= end || start >= args.len() {
break;
}
process_cmd(cx, &args[start..end].join(" "), event)?;
}
Ok(())
}

pub fn process_cmd(
cx: &mut compositor::Context,
input: &str,
event: PromptEvent,
) -> anyhow::Result<()> {
let input = expand_args(cx.editor, input);
let parts = input.split_whitespace().collect::<Vec<&str>>();
if parts.is_empty() {
return Ok(());
}

// If command is numeric, interpret as line number and go there.
if parts.len() == 1 && parts[0].parse::<usize>().ok().is_some() {
if let Err(e) = typed::goto_line_number(cx, &[Cow::from(parts[0])], event) {
cx.editor.set_error(format!("{}", e));
return Err(e);
}
return Ok(());
}

// Handle typable commands
if let Some(cmd) = typed::TYPABLE_COMMAND_MAP.get(parts[0]) {
let shellwords = shellwords::Shellwords::from(input.as_ref());
let args = shellwords.words();

if let Err(e) = (cmd.fun)(cx, &args[1..], event) {
cx.editor.set_error(format!("{}", e));
return Err(e);
}
} else if event == PromptEvent::Validate {
cx.editor
.set_error(format!("no such command: '{}'", parts[0]));
}
Comment on lines +2197 to +2200
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this return an error too?

Ok(())
}

pub const TYPABLE_COMMAND_LIST: &[TypableCommand] = &[
TypableCommand {
name: "quit",
Expand Down Expand Up @@ -2240,6 +2301,13 @@ pub const TYPABLE_COMMAND_LIST: &[TypableCommand] = &[
fun: run_shell_command,
completer: Some(completers::directory),
},
TypableCommand {
name: "commands",
aliases: &["cmds"],
doc: "Run commands together, use && to sepearte them",
fun: cmds,
completer: Some(completers::filename),
},
];

pub static TYPABLE_COMMAND_MAP: Lazy<HashMap<&'static str, &'static TypableCommand>> =
Expand Down Expand Up @@ -2317,31 +2385,7 @@ pub(super) fn command_mode(cx: &mut Context) {
}
}, // completion
move |cx: &mut compositor::Context, input: &str, event: PromptEvent| {
let parts = input.split_whitespace().collect::<Vec<&str>>();
if parts.is_empty() {
return;
}

// If command is numeric, interpret as line number and go there.
if parts.len() == 1 && parts[0].parse::<usize>().ok().is_some() {
if let Err(e) = typed::goto_line_number(cx, &[Cow::from(parts[0])], event) {
cx.editor.set_error(format!("{}", e));
}
return;
}

// Handle typable commands
if let Some(cmd) = typed::TYPABLE_COMMAND_MAP.get(parts[0]) {
let shellwords = Shellwords::from(input);
let args = shellwords.words();

if let Err(e) = (cmd.fun)(cx, &args[1..], event) {
cx.editor.set_error(format!("{}", e));
}
} else if event == PromptEvent::Validate {
cx.editor
.set_error(format!("no such command: '{}'", parts[0]));
}
let _ = process_cmd(cx, input, event);
},
);
prompt.doc_fn = Box::new(|input: &str| {
Expand All @@ -2363,3 +2407,58 @@ pub(super) fn command_mode(cx: &mut Context) {
prompt.recalculate_completion(cx.editor);
cx.push_layer(Box::new(prompt));
}

fn expand_args<'a>(editor: &mut Editor, args: &'a str) -> Cow<'a, str> {
let reg = Regex::new(r"%(\w+)\s*\{(.*)").unwrap();
reg.replace(args, |caps: &regex::Captures| {
let remaining = &caps[2];
let end = find_first_open_right_braces(remaining);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we auto-complete for users? I think %sh{echo hello shouldn't work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, above command won't expand and will be treated literately . The auto-complete is a very nice to have feature, how about we adding it after this PR.

let exp = expand_args(editor, &remaining[..end]);
let next = expand_args(editor, remaining.get(end + 1..).unwrap_or(""));
let (_view, doc) = current_ref!(editor);
format!(
"{} {}",
match &caps[1] {
"val" => match exp.trim() {
"filename" => doc.path().and_then(|p| p.to_str()).unwrap_or("").to_owned(),
"dirname" => doc
.path()
.and_then(|p| p.parent())
.and_then(|p| p.to_str())
.unwrap_or("")
.to_owned(),
_ => "".into(),
},
"sh" => {
let shell = &editor.config().shell;
if let Ok((output, _)) = shell_impl(shell, &exp, None) {
output.trim().into()
} else {
"".into()
}
}
_ => "".into(),
},
next
)
QiBaobin marked this conversation as resolved.
Show resolved Hide resolved
})
}

fn find_first_open_right_braces(str: &str) -> usize {
let mut left_count = 1;
for (i, &b) in str.as_bytes().iter().enumerate() {
match char::from_u32(b as u32) {
Comment on lines +2891 to +2892
Copy link
Contributor

@pickfire pickfire Jan 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Shouldn't we iterate over char directly instead? I think unicode might crash in this case when we are reading bytes within multiple bytes char.

Also, the count with unicode characters is incorrect now, as_bytes may increment i more times than the actual character length str.len.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the b is a byte so that this won't crash.As we only use ascii chars in our patterns below, I didn't treat them as Unicode char, this shall work until we introduce other Unicode pattern beside %val etc.

Copy link
Contributor

@g-re-g g-re-g Jan 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

If these patterns are coming from user input there's always a chance that they can be multi-byte, or even multi code-point "characters". Because of that you might want the parser to be aware / tolerant of it.

If that is the case then String.chars probably still isn't enough and you'll need something more unicode aware if you're interested in knowing a particular characters index as a human would see it. See this note from the rust std library:

let y = "y̆";
let mut chars = y.chars();
assert_eq!(Some('y'), chars.next()); // not 'y̆'

The typical crate I've used / seen used is https://crates.io/crates/unicode-segmentation . Specifically for this implementation you could use grapheme_indices https://unicode-rs.github.io/unicode-segmentation/unicode_segmentation/trait.UnicodeSegmentation.html#tymethod.grapheme_indices .

Copy link
Contributor

@trink trink Jan 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The returned index will always be in bounds (the enumerator and len are both byte based). Trying something like the following doesn't cause any issues
sh echo %val{𒀀𒀀𒀀𒀀𒀀𒀀𒀀} // 4 byte char f0928080

I think this should remain as is.

However, downstream I am seeing something off in shellwords (some multi-byte characters are lost in the generated argument list, I am investigating it now but it is unrelated to this PR)
edit: #5738

Some('}') => {
left_count -= 1;
if left_count == 0 {
return i;
}
}
Some('{') => {
left_count += 1;
}
_ => {}
}
}
str.len()
}