Skip to content

Conversation

vezwork
Copy link
Collaborator

@vezwork vezwork commented Aug 29, 2025

Problem

Snippets were being filtered from completions because of a position miscalculation.

The itemFrom function was not correct, it was doing something clever that it is not quite right.

itemFrom should calculate how many characters into the document the start of a completion is. It did this by calculating the length of the completion and subtracting that from the text cursor's position. The way it was calculating the length of the completion was not correct. The completion gives its positions in the context of the cvContext.code, which is different from the positions in the actual document; For one thing, the line containing the language string (like {r}) is removed from cvContext.code. So, we can't use the positions the completion gives us directly. That is why itemFrom was cleverly not using the positions, but instead using the positions to calculate the length of the completion.

But, for example, if you are typing libr, a completion for the library snippet will say that its insertion character position is from 0 to 7. What's up with that? For whatever reason, since your cursor is at the end of a line with 4 characters, the completion pretends that it can replace 3 more non-existent characters in the rest of the line. If there were any characters in the line after libr, even just whitespace like libr , then the completion would instead say that its insertion character position is from 0 to 4.

This PR fixes the issue by snapping the ending position of a completion to the end of its line in cvContext.code.

After I fixed this issue, I was able to easily insert the library snippet in R code, and this uncovered a bug in snippets in codemirror autocomplete that I reported here codemirror/dev#1607.

@vezwork vezwork changed the base branch from main to fix/completions August 29, 2025 20:18
@vezwork
Copy link
Collaborator Author

vezwork commented Sep 2, 2025

@marijnh fixed the codemirror/autocomplete issue and made a release of it. I spent a couple hours this morning on this PR's branch attempting to upgrade our codemirror/autocomplete version. I was not successful (facing Quarto monorepo problems that I don't understand).

I feel that this PR is blocked on getting this autocomplete fix in. If we are going to fix it so that snippet completions show up properly, snippets should work.

I will have to get help figuring out how to upgrade the package.

@vezwork
Copy link
Collaborator Author

vezwork commented Sep 2, 2025

Here's a description of the issues I was running into while upgrading: I used yarn upgrade-interactive --latest in the repo root to upgrade various codemirror packages to their latest versions: view, state, autocomplete and maybe another one or two. This changed packages/editor-codemirror/package.json to reflect the updated versions and updated the packages in packages/editor-codemirror/node_modules. The problem is that the root node_modules still contains old versions of these packages, and that causes type and build errors in files inside packages/editor-codemirror, such as completions.ts.

I tried deleting the root level yarn.lock and node_modules and doing a fresh yarn install, and this fixes the codemirror package issues, but seems to cause many other issues. The first issue I encountered going down this path was type issues with markdown-it-attrs. I commented code related to that issue out and ran into another issue. I am afraid.

I do not understand why the root node_modules still contains old versions of these packages. These packages are not referenced in any other places other than packages/editor-codemirror/package.json. Why are these packages installed in both the root node_modules and editor-codemirror node_modules? What is going on??

@marijnh
Copy link

marijnh commented Sep 2, 2025

Stop using yarn. It's a broken tool that will duplicate dependencies all over the place when you upgrade something. You can blow away your package lock on every update to work around that, or switch back to npm, which is a more reliable tool.

@cscheid
Copy link
Contributor

cscheid commented Sep 3, 2025

I imagine part of what's happening here is that this is a https://turborepo.com/ monorepo, and yarn might be getting extra confused in the process?

(I would happily switch us all away to better tooling if the benefits are there, but I would really like to not to have to switch again in a year. So I think adding some friction here and thinking half a dozen times before pulling the trigger is worthwhile.)

@vezwork
Copy link
Collaborator Author

vezwork commented Sep 3, 2025

I hadn't considered that yarn has problems, I thought I was doing something wrong. Here's a yarn issue that seems to describe a related, if not the same, problem as the problem I was running into.

I am running an old yarn version. I suppose the smallest thing to try for now is to upgrade yarn (two major versions, I'm on 1.22.22 and that thread ends in a claim that this problem is fixed in 3.1.1) and see if that works and helps.

@vezwork vezwork changed the base branch from fix/completions to main September 3, 2025 18:48
@vezwork vezwork force-pushed the fix/completions-snippets branch from 52d3eda to df7129d Compare September 3, 2025 18:48
@vezwork vezwork force-pushed the fix/completions-snippets branch from df7129d to 36c940e Compare September 3, 2025 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants