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

feat: add a way to spawn populated LineEditor #6007

Closed
wants to merge 2 commits into from
Closed

feat: add a way to spawn populated LineEditor #6007

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b78a3b8
feat: add a way to spawn populated LineEditor
ekorchmar Aug 21, 2024
d1f2050
chore: rename new parameter and simplify interface
ekorchmar Sep 22, 2024
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
3 changes: 3 additions & 0 deletions config/src/keyassignment.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,9 @@ pub struct QuickSelectArguments {
#[derive(Debug, Clone, PartialEq, FromDynamic, ToDynamic)]
pub struct PromptInputLine {
pub action: Box<KeyAssignment>,
/// Optional label to pre-fill the input line with
#[dynamic(default)]
pub initial_value: Option<String>,
/// Descriptive text to show ahead of prompt
#[dynamic(default)]
pub description: String,
Expand Down
6 changes: 5 additions & 1 deletion docs/config/lua/keyassignment/PromptInputLine.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ from the user.
When the user enters the line, emits an event that allows you to act
upon the input.

`PromptInputLine` accepts three fields:
`PromptInputLine` accepts four fields:

* `description` - the text to show at the top of the display area. You may
embed escape sequences and/or use [wezterm.format](../wezterm/format.md).
Expand All @@ -20,6 +20,9 @@ upon the input.
anything, or CTRL-C to cancel the input.
* `prompt` - the text to show as the prompt. You may embed escape sequences
and/or use [wezterm.format](../wezterm/format.md). Defaults to: `"> "`. {{since('nightly', inline=True)}}
* `initial_value` - optional. If provided, the initial content of the input
field will be set to this value. The user may edit it prior to submitting
the input.
Copy link
Owner

Choose a reason for hiding this comment

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

Let's tag this with the version it was introduced with:

Suggested change
the input.
the input. {{since('nightly', inline=True)}}


## Example of interactively renaming the current tab

Expand All @@ -34,6 +37,7 @@ config.keys = {
mods = 'CTRL|SHIFT',
action = act.PromptInputLine {
description = 'Enter new name for tab',
initial_value = 'My Tab Name',
action = wezterm.action_callback(function(window, pane, line)
-- line will be `nil` if they hit escape without entering anything
-- An empty string if they just hit enter
Expand Down
1 change: 1 addition & 0 deletions docs/config/lua/window-events/augment-command-palette.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ wezterm.on('augment-command-palette', function(window, pane)

action = act.PromptInputLine {
description = 'Enter new name for tab',
initial_value = 'My Tab Name',
action = wezterm.action_callback(function(window, pane, line)
if line then
window:active_tab():set_title(line)
Expand Down
1 change: 0 additions & 1 deletion termwiz/src/lineedit/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -811,7 +811,6 @@ impl<'term> LineEditor<'term> {
}

fn read_line_impl(&mut self, host: &mut dyn LineEditorHost) -> Result<Option<String>> {
self.line.clear();
Comment on lines 813 to -814
Copy link
Owner

Choose a reason for hiding this comment

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

ah, I didn't spot this initially. I think this is a big semantic change that breaks behavior. You can probably see this in the debug overlay (ctrl-shift-L) if you type in foo and hit enter. Does it cause foo to show up in the next line?

What I think should be done here is to adjust this method:

fn read_line_impl(&mut self,
    host: &mut dyn LineEditorHost,
    initial_value: Option<&str>
) -> Result<Option<String>> {
    self.line.clear();
    if let Some(value) = initial_value {
        self.line.set_line_and_cursor(value, value.len());
    }

Then, refactor:

pub fn read_line(&mut self, host: &mut dyn LineEditorHost) -> Result<Option<String>>

into

pub fn read_line_with_optional_initial_value(&mut self, host: &mut dyn LineEditorHost, initial_value: Option<&str>) -> Result<Option<String>>

that calls the above function:

...
let res = self.read_line_impl(host, initial_value);
...

Then finally add back the original method, but make it call the refactored one:

pub fn read_line(&mut self, host: &mut dyn LineEditorHost) -> Result<Option<String>> {
   self.read_line_with_optional_initial_value(host, None)
}

Note that the public methods on this struct need to have doc comments because they are part of the public API that is published to https://docs.rs/termwiz/latest/termwiz/lineedit/struct.LineEditor.html

Copy link
Contributor Author

@ekorchmar ekorchmar Sep 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think this is a big semantic change that breaks behavior. You can probably see this in the debug overlay (ctrl-shift-L) if you type in foo and hit enter. Does it cause foo to show up in the next line?

From what I can test, it does not appear as if removal of this line causes old contents to appear anywhere; I tested various combinations of launching REPL, custom editor, pressing enter, cancelling input with Esc and Ctrl+C and other things.

If I read the code correctly, in every instance where a LineEditor struct is spawned, it's read_line method is only ever called once, with no loops. As such, the "clear" function does not actually do anything: it only ever sees an already empty, freshly created line.

Copy link
Owner

Choose a reason for hiding this comment

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

The termwiz crate is a public export and used by others, so just looking at the usage within wezterm is not sufficient to determine that nothing is impacted by this change.

I went ahead and merged this with my suggested changes.

Thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This makes sense. Thank you for reviewing this

self.history_pos = None;
self.bottom_line = None;
self.clear_completion();
Expand Down
3 changes: 3 additions & 0 deletions wezterm-gui/src/overlay/prompt.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ pub fn show_line_prompt_overlay(
let mut host = PromptHost::new();
let mut editor = LineEditor::new(&mut term);
editor.set_prompt(&args.prompt);
if let Some(value) = &args.initial_value {
editor.set_line_and_cursor(value, value.len());
}
let line = editor.read_line(&mut host)?;
Comment on lines +71 to 74
Copy link
Owner

Choose a reason for hiding this comment

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

with the above changes, this would become:

Suggested change
if let Some(value) = &args.initial_value {
editor.set_line_and_cursor(value, value.len());
}
let line = editor.read_line(&mut host)?;
let line = editor.read_line_with_optional_initial_value(&mut host, args.initial_value.as_deref())?;


promise::spawn::spawn_into_main_thread(async move {
Expand Down