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

Theme preview doesn't return theme to normal when navigating command history #3620

Closed
Omnikar opened this issue Aug 31, 2022 · 1 comment · Fixed by #3644
Closed

Theme preview doesn't return theme to normal when navigating command history #3620

Omnikar opened this issue Aug 31, 2022 · 1 comment · Fixed by #3644
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug

Comments

@Omnikar
Copy link
Contributor

Omnikar commented Aug 31, 2022

Summary

The theme preview when using :theme will keep the previewed theme if you switch away from the command using history navigation.

Reproduction Steps

  1. Enter any (not necessarily valid) command, that is not a :theme command, such as :abc<ret>.
  2. Type :theme and hover the completion over a theme other than the currently selected one, thus previewing it.
  3. Press the up arrow key, causing the command to switch to the one entered in step 1.
  4. Press <esc>, exiting the command prompt.

The previewed theme will persist after the command is switched away from using the up arrow key.

Helix log

Nothing relevant

Platform

MacOS

Terminal Emulator

iTerm2 v3.4.16

Helix Version

Helix 22.08

@Omnikar Omnikar added the C-bug Category: This is a bug label Aug 31, 2022
@jharrilim
Copy link
Contributor

I can make the following changes in a PR, but the gist of it is we need to:

  1. Ensure that history navigation aborts the previous command. This can be done by changing

ctrl!('p') | key!(Up) => {
if let Some(register) = self.history_register {
self.change_history(cx, register, CompletionDirection::Backward);
(self.callback_fn)(cx, &self.line, PromptEvent::Update);
}
}
ctrl!('n') | key!(Down) => {
if let Some(register) = self.history_register {
self.change_history(cx, register, CompletionDirection::Forward);
(self.callback_fn)(cx, &self.line, PromptEvent::Update);
}
}

to:

            ctrl!('p') | key!(Up) => {
                if let Some(register) = self.history_register {
                    (self.callback_fn)(cx, &self.line, PromptEvent::Abort);
                    let register = cx.editor.registers.get_mut(register);
                    self.change_history(register.read(), CompletionDirection::Backward);
                    (self.callback_fn)(cx, &self.line, PromptEvent::Update);
                }
            }
            ctrl!('n') | key!(Down) => {
                if let Some(register) = self.history_register {
                    (self.callback_fn)(cx, &self.line, PromptEvent::Abort);
                    let register = cx.editor.registers.get_mut(register);
                    self.change_history(register.read(), CompletionDirection::Forward);
                    (self.callback_fn)(cx, &self.line, PromptEvent::Update);
                }
            }
  1. We need to ensure the completion items are updated too. This can be done by using the following code in the above functions after change_history:

self.completion = cx
.editor
.registers
.inner()
.iter()
.map(|(ch, reg)| {
let content = reg
.read()
.get(0)
.and_then(|s| s.lines().next().to_owned())
.unwrap_or_default();
(0.., format!("{} {}", ch, &content).into())
})
.collect();
self.next_char_handler = Some(Box::new(|prompt, c, context| {
prompt.insert_str(
context
.editor
.registers
.read(c)
.and_then(|r| r.first())
.map_or("", |r| r.as_str()),
context.editor,
);
}));
(self.callback_fn)(cx, &self.line, PromptEvent::Update);


Unrelated to the issue above but discovered along the way: The preview isn't rolled back if you hold backspace and then enter a new command.

eg.

  1. type :theme acme
  2. hold backspace until it's just :
  3. type :open README.md and press enter

This requires a change in this part:

PromptEvent::Update => {
if let Some(theme_name) = args.first() {
if let Ok(theme) = cx.editor.theme_loader.load(theme_name) {
if !(true_color || theme.is_16_color()) {
bail!("Unsupported theme: theme requires true color support");
}
cx.editor.set_theme_preview(theme);
};
};
}

to:

        PromptEvent::Update => {
            if args.is_empty() {
                cx.editor.unset_theme_preview();
            } else if let Some(theme_name) = args.first() {
                if let Ok(theme) = cx.editor.theme_loader.load(theme_name) {
                    if !(true_color || theme.is_16_color()) {
                        bail!("Unsupported theme: theme requires true color support");
                    }
                    cx.editor.set_theme_preview(theme);
                };
            };
        }

@the-mikedavis the-mikedavis added the A-helix-term Area: Helix term improvements label Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants