From 7eeb37262aa55b207c7df1266c8276b96457dbd3 Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Tue, 13 Aug 2024 20:50:51 +0200 Subject: [PATCH] assistant: Fix nested slash command rendering (#16173) /rant on We have this issue where if a prompt starts with a slash command (e.g. /workflow), the rendering is a bit messed up. The nested slash command gets picked up as the parent of a command that includes it (/prompt). This is due to how we parse slash commands; their output is obtained asynchronously. When we run `/prompt "My prompt"` whose contents are `/workflow`, we first include the prompt content verbatim and then reparse the whole buffer, picking up /workflow as a new command (as if it was typed by an user). The problem with that is that the range of parent /prompt does not include the expanded range of a /workflow; in fact, after doing full expansion of "My prompt", we lose track of the parent-children relationship of these two slash commands and treat them as if top-level user prompt was `/workflow/prompt "My prompt"` and not `/prompt "My prompt"` (which, by the way, would not be parsable for us). The "proper" fix would be to update the parent range whenever we parse a new children within it. We could do that. But then, the question is; what do we gain from it? Slash command output is put behind a crease, which is fundamentally a fold. Given "My prompt", we'd have to put two fold indicators on a single line even if the ranges were set up correctly. So that merely moves the target elsewhere into yet another issue. Even if we did solve two-fold problem somehow (by e.g. sorting same-line folds by the end point), we would still be stuck with suboptimal render. What do we gain from all that anyways? Proper handling of a relatively obscure (although - at the same time - quite common) edge case which may as well be handled by having /prompt insert a new line if there's a slight chance that the edge case could occur. And that hacky, "inproper" solution is what this PR does; in fact, it's not the first time it was done, as /default also has the same issue which it solves in precisely the same manner. /rant off Release Notes: - N/A --- crates/assistant/src/slash_command/prompt_command.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/assistant/src/slash_command/prompt_command.rs b/crates/assistant/src/slash_command/prompt_command.rs index 1b2769add8..0eba5a6dc1 100644 --- a/crates/assistant/src/slash_command/prompt_command.rs +++ b/crates/assistant/src/slash_command/prompt_command.rs @@ -77,6 +77,11 @@ impl SlashCommand for PromptSlashCommand { }); cx.foreground_executor().spawn(async move { let mut prompt = prompt.await?; + + if prompt.starts_with('/') { + // Prevent an edge case where the inserted prompt starts with a slash command (that leads to funky rendering). + prompt.insert(0, '\n'); + } if prompt.is_empty() { prompt.push('\n'); }