zk-org / zk

A plain text note-taking assistant
https://zk-org.github.io/zk/
GNU General Public License v3.0
1.64k stars 119 forks source link

Completion provides no results unless `[[` are typed #444

Open WhyNotHugo opened 1 month ago

WhyNotHugo commented 1 month ago

Check if applicable

Describe the bug

When I manually invoke auto-completion, it won't provide any suggestions for inserting links.

Moreover, if using markdown style links, I still need to type [[ before the completion provider yields any results.

How to reproduce?

  1. Invoke your text editor's autocompletion
  2. You'll see no suggestions for inserting links

OR

  1. Configure your notebook to use markdown style links.
  2. Type [ and (optionally) manually trigger autocompletion.
  3. You'll see no suggestions for inserting links

The root cause here is the completion provider only yields results in the two preceding characters are [[:

https://github.com/zk-org/zk/blob/4a51e398ae4572b2a1338fd9017ceeff30e5ffda/internal/adapter/lsp/server.go#L647

zk configuration

[notebook]
dir = "~/zk"

[note]
filename = "{{id}}"

[format.markdown]
hashtags = true
link-format = "[{{title}}]({{rel-path}})"

[lsp.diagnostics]
dead-link = "error"

### Environment

```bash
zk-0.14.1-r2
system: Linux 6.10.3-0-edge x86_64 Linux
tjex commented 1 month ago

Typing [[ is the default behaviour to trigger link completion though (for markdown links also, which is a little confusing). Regular markdown link completion ([title]( <-completion starts) is supposed to work, but has been found not to in #435, see context here in discussions.

As far as I'm aware, it was never designed that completion should work without any trigger / syntax context (e.g, manual completion).

But :ZkInsertLink suffices for link insertion in place of manual completion, or?

So I'm not sure if this is a "bug", but maybe edging more on a feature request?

I.e, that completion can be triggered manually and the LSP returns a list of links to insert?

mickael-menu commented 1 month ago

I had a quick look at the code and I think @WhyNotHugo is correct. An LSP client sends a "trigger kind" which can be invoked if the user hit a keyboard shortcut or trigger-character if known characters are typed, like [[.

The regression happened in this PR, but maybe @Rahlir had a good reason to check for [[ there. Do you remember why you changed the buildInvokedCompletionList implementation?

Rahlir commented 4 weeks ago

Hmm, it looks like this was part of the fix where previously the manual completion only worked with the # character, as you can see in this diff. So I don't think it is a regression per-say. However, I think I might have forgotten to check for links other than the wiki links. I will have a look at this this week and fix it, thanks for reporting this.

@tjex: Could you assign this issue to me so that I don't forget? Thanks.

mickael-menu commented 4 weeks ago

@Rahlir You're right, it couldn't have worked before either. I guess we could have a fallback on auto-completing links unless we're inside a tag (#).

WhyNotHugo commented 4 weeks ago

When a completion is manually requested, it's fine to suggest both links and tags.

In case of [[ (with wiki-style links) or [ (with markdownlinks), suggest only links.

In case of $TAG_PREFIX, suggest only tags.

Additionally, [[/[ should trigger completion.

Rahlir commented 4 weeks ago

When a completion is manually requested, it's fine to suggest both links and tags.

Interesting, so you mean that if you trigger manual completion when typing for instance note<cursor here> you would like to generate completion for both tags and notes containing word note in them? Because the intent of my previous commit was to only generate completions if the text contains either # (which would mean that the user is trying to complete tags) or [[ (which would mean that the user is trying to complete link - here was my mistake of forgetting other types of links). What you are suggesting differs from my intent. Although I am happy to do it this way. Although I am not sure what is the "more correct approach". What are your thoughts @mickael-menu @tjex ?

mickael-menu commented 4 weeks ago

There are also other types of tags, so maybe it's safer to just offer completion of everything, when we don't have a context.

tjex commented 3 weeks ago

I would agree. But I think links should take a higher priority in the LSP completion list (if there's currently support for that within our LSP implementation, or it's relatively trivial to implement). As most of the time, without any textual context, I would think users will be wanting to insert a link.

I also just checked, and I'm only getting tag completion for YAML tags. Hash or colon tags don't return anything. With colon tags set in .zk/config.toml

Caret in YAML array.

  1 ---
2   tags: [|]
  1 ---   ╭─────────────╮
  2       │ foo 󰉿 [LSP] │
  3       ╰─────────────╯

Carret after :.

  4 ---
  3 tags: []
  2 ---
  1
5   :|
~   ╭──────────────╮
~   │ tags 󰉿 [buf] │
~   ╰──────────────╯

So I think this part of the implementation from the PR needs some attention as well?

https://github.com/zk-org/zk/blob/4a51e398ae4572b2a1338fd9017ceeff30e5ffda/internal/adapter/lsp/document.go#L356C1-L358C3