Skip to content

Commit

Permalink
feat: prefer to guide coding by signature help (#874)
Browse files Browse the repository at this point in the history
* feat: prefer to guide coding by signature help

* test: update markdown description

* test: update configuration for testing
  • Loading branch information
Myriad-Dreamin authored Nov 21, 2024
1 parent 1ba6b6c commit 67e659a
Show file tree
Hide file tree
Showing 13 changed files with 102 additions and 38 deletions.
31 changes: 22 additions & 9 deletions crates/tinymist-query/src/analysis/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,21 +146,34 @@ impl Analysis {
AllocStats::report(self)
}

/// Get configured trigger suggest command.
pub fn trigger_suggest(&self, context: bool) -> Option<&'static str> {
(self.completion_feat.trigger_suggest && context).then_some("editor.action.triggerSuggest")
}

/// Get configured trigger parameter hints command.
pub fn trigger_parameter_hints(&self, context: bool) -> Option<&'static str> {
(self.completion_feat.trigger_parameter_hints && context)
.then_some("editor.action.triggerParameterHints")
}

/// Get configured trigger named completion command.
pub fn trigger_named_completion(&self, context: bool) -> Option<&'static str> {
(self.completion_feat.trigger_named_completion && context)
.then_some("tinymist.triggerNamedCompletion")
/// Get configured trigger after snippet command.
///
/// > VS Code doesn't do that... Auto triggering suggestion only happens on
/// > typing (word starts or trigger characters). However, you can use
/// > editor.action.triggerSuggest as command on a suggestion to "manually"
/// > retrigger suggest after inserting one
pub fn trigger_on_snippet(&self, context: bool) -> Option<&'static str> {
if !self.completion_feat.trigger_on_snippet_placeholders {
return None;
}

(self.completion_feat.trigger_suggest && context).then_some("editor.action.triggerSuggest")
}

/// Get configured trigger on positional parameter hints command.
pub fn trigger_on_snippet_with_param_hint(&self, context: bool) -> Option<&'static str> {
if !self.completion_feat.trigger_on_snippet_placeholders {
return self.trigger_parameter_hints(context);
}

(self.completion_feat.trigger_suggest_and_parameter_hints && context)
.then_some("tinymist.triggerSuggestAndParameterHints")
}
}

Expand Down
9 changes: 9 additions & 0 deletions crates/tinymist-query/src/completion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,15 @@ impl StatefulRequest for CompletionRequest {
ctx: &mut LocalContext,
doc: Option<VersionedDocument>,
) -> Option<Self::Response> {
// These trigger characters are for completion on positional arguments,
// which follows the configuration item
// `tinymist.completion.triggerOnSnippetPlaceholders`.
if matches!(self.trigger_character, Some('(' | ',' | ':'))
&& !ctx.analysis.completion_feat.trigger_on_snippet_placeholders
{
return None;
}

let doc = doc.as_ref().map(|doc| doc.document.as_ref());
let source = ctx.source_by_path(&self.path).ok()?;
let (cursor, deref_target) = ctx.deref_syntax_at_(&source, self.position, 0)?;
Expand Down
9 changes: 8 additions & 1 deletion crates/tinymist-query/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ pub use tinymist_world::{LspUniverse, LspUniverseBuilder};
use typst_shim::syntax::LinkedNodeExt;

use crate::syntax::find_module_level_docs;
use crate::LspWorldExt;
use crate::{
analysis::Analysis, prelude::LocalContext, typst_to_lsp, LspPosition, PositionEncoding,
VersionedDocument,
};
use crate::{CompletionFeat, LspWorldExt};

type CompileDriver<C> = CompileDriverImpl<C, tinymist_world::LspCompilerFeat>;

Expand Down Expand Up @@ -74,6 +74,13 @@ pub fn run_with_ctx<T>(

let mut ctx = Arc::new(Analysis {
remove_html: !supports_html,
completion_feat: CompletionFeat {
trigger_on_snippet_placeholders: true,
trigger_suggest: true,
trigger_parameter_hints: true,
trigger_suggest_and_parameter_hints: true,
..Default::default()
},
..Analysis::default()
})
.snapshot(w);
Expand Down
2 changes: 1 addition & 1 deletion crates/tinymist-query/src/ty/iface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ impl<'a> Iface<'a> {
}

pub trait IfaceChecker: TyCtx {
fn check(&mut self, sig: Iface, args: &mut IfaceCheckContext, pol: bool) -> Option<()>;
fn check(&mut self, iface: Iface, ctx: &mut IfaceCheckContext, pol: bool) -> Option<()>;
}

impl Ty {
Expand Down
5 changes: 1 addition & 4 deletions crates/tinymist-query/src/upstream/complete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -885,10 +885,7 @@ impl<'a> CompletionContext<'a> {
apply: Some(snippet.into()),
detail: Some(docs.into()),
label_detail: None,
// VS Code doesn't do that... Auto triggering suggestion only happens on typing (word
// starts or trigger characters). However, you can use editor.action.triggerSuggest as
// command on a suggestion to "manually" retrigger suggest after inserting one
command: self.ctx.analysis.trigger_suggest(snippet.contains("${")),
command: self.ctx.analysis.trigger_on_snippet(snippet.contains("${")),
..Completion::default()
});
}
Expand Down
27 changes: 17 additions & 10 deletions crates/tinymist-query/src/upstream/complete/ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,19 @@ pub struct PostfixSnippet {
#[derive(Default, Debug, Clone, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct CompletionFeat {
/// Whether to trigger suggest completion, a.k.a. auto-completion.
/// Whether to trigger completions on arguments (placeholders) of snippets.
#[serde(default)]
pub trigger_on_snippet_placeholders: bool,
/// Whether supports trigger suggest completion, a.k.a. auto-completion.
#[serde(default)]
pub trigger_suggest: bool,
/// Whether to trigger named parameter completion.
pub trigger_named_completion: bool,
/// Whether to trigger parameter hint, a.k.a. signature help.
/// Whether supports trigger parameter hint, a.k.a. signature help.
#[serde(default)]
pub trigger_parameter_hints: bool,
/// Whether supports trigger the command combining suggest and parameter
/// hints.
#[serde(default)]
pub trigger_suggest_and_parameter_hints: bool,

/// Whether to enable postfix completion.
pub postfix: Option<bool>,
Expand Down Expand Up @@ -332,7 +339,7 @@ impl<'a> CompletionContext<'a> {
label_detail,
apply: Some("".into()),
// range: Some(range),
command: self.ctx.analysis.trigger_parameter_hints(true),
command: self.ctx.analysis.trigger_on_snippet_with_param_hint(true),
..Default::default()
};
let fn_feat = FnCompletionFeat::default().check(kind_checker.functions.iter());
Expand Down Expand Up @@ -475,7 +482,7 @@ impl<'a> CompletionContext<'a> {
let base = Completion {
kind: CompletionKind::Func,
label_detail,
command: self.ctx.analysis.trigger_parameter_hints(true),
command: self.ctx.analysis.trigger_on_snippet_with_param_hint(true),
..Default::default()
};

Expand Down Expand Up @@ -683,11 +690,11 @@ struct ScopeChecker<'a>(&'a mut Defines, &'a mut LocalContext);
impl<'a> IfaceChecker for ScopeChecker<'a> {
fn check(
&mut self,
sig: Iface,
_args: &mut crate::ty::IfaceCheckContext,
iface: Iface,
_ctx: &mut crate::ty::IfaceCheckContext,
_pol: bool,
) -> Option<()> {
match sig {
match iface {
// dict is not importable
Iface::Dict(..) | Iface::Value { .. } => {}
Iface::Element { val, .. } => {
Expand Down Expand Up @@ -1094,7 +1101,7 @@ fn type_completion(
apply: Some(eco_format!("{}: ${{}}", f)),
label_detail: p.ty.describe(),
detail: docs.map(Into::into),
command: ctx.ctx.analysis.trigger_named_completion(true),
command: ctx.ctx.analysis.trigger_on_snippet_with_param_hint(true),
..Completion::default()
});
}
Expand Down
21 changes: 17 additions & 4 deletions crates/tinymist/src/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,11 @@ impl Initializer for SuperInit {
// position_encoding: Some(cc.position_encoding.into()),
hover_provider: Some(HoverProviderCapability::Simple(true)),
signature_help_provider: Some(SignatureHelpOptions {
trigger_characters: Some(vec!["(".to_string(), ",".to_string()]),
trigger_characters: Some(vec![
String::from("("),
String::from(","),
String::from(":"),
]),
retrigger_characters: None,
work_done_progress_options: WorkDoneProgressOptions {
work_done_progress: None,
Expand Down Expand Up @@ -356,23 +360,32 @@ impl Config {
pub fn update_by_map(&mut self, update: &Map<String, JsonValue>) -> anyhow::Result<()> {
macro_rules! assign_config {
($( $field_path:ident ).+ := $bind:literal?: $ty:ty) => {
let v = try_(|| <$ty>::deserialize(update.get($bind)?).ok());
let v = try_deserialize::<$ty>(update, $bind);
self.$($field_path).+ = v.unwrap_or_default();
};
($( $field_path:ident ).+ := $bind:literal: $ty:ty = $default_value:expr) => {
let v = try_(|| <$ty>::deserialize(update.get($bind)?).ok());
let v = try_deserialize::<$ty>(update, $bind);
self.$($field_path).+ = v.unwrap_or_else(|| $default_value);
};
}

fn try_deserialize<T: serde::de::DeserializeOwned>(
map: &Map<String, JsonValue>,
key: &str,
) -> Option<T> {
T::deserialize(map.get(key)?)
.inspect_err(|e| log::warn!("failed to deserialize {key:?}: {e}"))
.ok()
}

assign_config!(semantic_tokens := "semanticTokens"?: SemanticTokensMode);
assign_config!(formatter_mode := "formatterMode"?: FormatterMode);
assign_config!(formatter_print_width := "formatterPrintWidth"?: Option<u32>);
assign_config!(support_html_in_markdown := "supportHtmlInMarkdown"?: bool);
assign_config!(completion := "completion"?: CompletionFeat);
assign_config!(completion.trigger_suggest := "triggerSuggest"?: bool);
assign_config!(completion.trigger_named_completion := "triggerNamedCompletion"?: bool);
assign_config!(completion.trigger_parameter_hints := "triggerParameterHints"?: bool);
assign_config!(completion.trigger_suggest_and_parameter_hints := "triggerSuggestAndParameterHints"?: bool);
self.compile.update_by_map(update)?;
self.compile.validate()
}
Expand Down
6 changes: 6 additions & 0 deletions editors/neovim/Configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,12 @@ Set the print width for the formatter, which is a **soft limit** of characters p
- **Type**: `number`
- **Default**: `120`

## `completion.triggerOnSnippetPlaceholders`

Whether to trigger completions on arguments (placeholders) of snippets. For example, `box` will be completed to `box(|)`, and server will request the editor (lsp client) to request completion after moving cursor to the placeholder in the snippet. Note: this has no effect if the editor doesn't support `editor.action.triggerSuggest` or `tinymist.triggerSuggestAndParameterHints` command. Hint: Restarting the editor is required to change this setting.

- **Type**: `boolean`

## `completion.postfix`

Whether to enable postfix code completion. For example, `[A].box|` will be completed to `box[A]|`. Hint: Restarting the editor is required to change this setting.
Expand Down
6 changes: 6 additions & 0 deletions editors/vscode/Configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,12 @@ Whether to handle drag-and-drop of resources into the editing typst document. No
- `disable`
- **Default**: `"enable"`

## `tinymist.completion.triggerOnSnippetPlaceholders`

Whether to trigger completions on arguments (placeholders) of snippets. For example, `box` will be completed to `box(|)`, and server will request the editor (lsp client) to request completion after moving cursor to the placeholder in the snippet. Note: this has no effect if the editor doesn't support `editor.action.triggerSuggest` or `tinymist.triggerSuggestAndParameterHints` command. Hint: Restarting the editor is required to change this setting.

- **Type**: `boolean`

## `tinymist.completion.postfix`

Whether to enable postfix code completion. For example, `[A].box|` will be completed to `box[A]|`. Hint: Restarting the editor is required to change this setting.
Expand Down
12 changes: 9 additions & 3 deletions editors/vscode/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -446,21 +446,27 @@
"disable"
]
},
"tinymist.completion.triggerOnSnippetPlaceholders": {
"title": "Trigger LSP Completion on Snippet Placeholders",
"markdownDescription": "Whether to trigger completions on arguments (placeholders) of snippets. For example, `box` will be completed to `box(|)`, and server will request the editor (lsp client) to request completion after moving cursor to the placeholder in the snippet. Note: this has no effect if the editor doesn't support `editor.action.triggerSuggest` or `tinymist.triggerSuggestAndParameterHints` command. Hint: Restarting the editor is required to change this setting.",
"type": "boolean",
"default": false
},
"tinymist.completion.postfix": {
"title": "Enable Postfix Code Completion",
"description": "Whether to enable postfix code completion. For example, `[A].box|` will be completed to `box[A]|`. Hint: Restarting the editor is required to change this setting.",
"markdownDescription": "Whether to enable postfix code completion. For example, `[A].box|` will be completed to `box[A]|`. Hint: Restarting the editor is required to change this setting.",
"type": "boolean",
"default": true
},
"tinymist.completion.postfixUfcs": {
"title": "Completion: Convert Field Access to Call",
"description": "Whether to enable UFCS-style completion. For example, `[A].box|` will be completed to `box[A]|`. Hint: Restarting the editor is required to change this setting.",
"markdownDescription": "Whether to enable UFCS-style completion. For example, `[A].box|` will be completed to `box[A]|`. Hint: Restarting the editor is required to change this setting.",
"type": "boolean",
"default": true
},
"tinymist.completion.postfixUfcsLeft": {
"title": "Completion: Convert Field Access to Call (Left Variant)",
"description": "Whether to enable left-variant UFCS-style completion. For example, `[A].table|` will be completed to `table(|)[A]`. Hint: Restarting the editor is required to change this setting.",
"markdownDescription": "Whether to enable left-variant UFCS-style completion. For example, `[A].table|` will be completed to `table(|)[A]`. Hint: Restarting the editor is required to change this setting.",
"type": "boolean",
"default": true
},
Expand Down
6 changes: 3 additions & 3 deletions editors/vscode/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export async function doActivate(context: ExtensionContext): Promise<void> {
const config = loadTinymistConfig();
// Inform server that we support named completion callback at the client side
config.triggerSuggest = true;
config.triggerNamedCompletion = true;
config.triggerSuggestAndParameterHints = true;
config.triggerParameterHints = true;
config.supportHtmlInMarkdown = true;
// Sets features
Expand Down Expand Up @@ -337,7 +337,7 @@ async function startClient(client: LanguageClient, context: ExtensionContext): P

// We would like to define it at the server side, but it is not possible for now.
// https://github.com/microsoft/language-server-protocol/issues/1117
commands.registerCommand("tinymist.triggerNamedCompletion", triggerNamedCompletion),
commands.registerCommand("tinymist.triggerSuggestAndParameterHints", triggerSuggestAndParameterHints),
);
// context.subscriptions.push
const provider = new SymbolViewProvider(context);
Expand Down Expand Up @@ -851,7 +851,7 @@ async function commandRunCodeLens(...args: string[]): Promise<void> {
}
}

function triggerNamedCompletion() {
function triggerSuggestAndParameterHints() {
vscode.commands.executeCommand("editor.action.triggerSuggest");
vscode.commands.executeCommand("editor.action.triggerParameterHints");
}
4 changes: 2 additions & 2 deletions tests/e2e/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ fn e2e() {
});

let hash = replay_log(&tinymist_binary, &root.join("neovim"));
insta::assert_snapshot!(hash, @"siphash128_13:3432d7a525d79141b5f306ddd9d18686");
insta::assert_snapshot!(hash, @"siphash128_13:6b94fdf913e670825d2b4daf6ef8c7f5");
}

{
Expand All @@ -385,7 +385,7 @@ fn e2e() {
});

let hash = replay_log(&tinymist_binary, &root.join("vscode"));
insta::assert_snapshot!(hash, @"siphash128_13:6ebe2f1dd20371d5bae4b585efbed731");
insta::assert_snapshot!(hash, @"siphash128_13:afe8f09344dbbd1ff501368282065404");
}
}

Expand Down
2 changes: 1 addition & 1 deletion tests/fixtures/initialization/vscode-1.87.2.json
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@
"trace": { "server": "off" },
"triggerSuggest": true,
"triggerParameterHints": true,
"triggerNamedCompletion": true,
"triggerSuggestAndParameterHints": true,
"supportHtmlInMarkdown": true,
"experimentalFormatterMode": "disable"
},
Expand Down

0 comments on commit 67e659a

Please sign in to comment.