Skip to content

fix: fix #180 - #198

Draft
jmbuhr wants to merge 4 commits into
mainfrom
fix/indents
Draft

fix: fix #180#198
jmbuhr wants to merge 4 commits into
mainfrom
fix/indents

Conversation

@jmbuhr

@jmbuhr jmbuhr commented Jan 9, 2025

Copy link
Copy Markdown
Owner

#180 appears to be related to indents, offsets and tabs vs. spaces.

@jmbuhr
jmbuhr marked this pull request as draft January 9, 2025 14:06
@ctdunc

ctdunc commented Feb 7, 2025

Copy link
Copy Markdown

Am experiencing the same issue---I would like to take a stab at fixing this. If you get a few seconds, could you elaborate a bit more on what you mean by "related to indents, offsets and tabs vs spaces"? I should have some time to spend on this in the next couple of weeks, would help me get off to a better start.

ty!

@jmbuhr

jmbuhr commented Feb 9, 2025

Copy link
Copy Markdown
Owner Author

Very cool! So https://github.com/jmbuhr/otter.nvim/blob/be6324e0987c4fab347784e602c00f17c5fc0bd7/tests/examples/07.html is a good example file to test this. If you activate otter in it and check the otter buffer created for the injected js (use :ls! to list hidden buffers, including the otter buffer or use https://github.com/jmbuhr/otter.nvim/blob/be6324e0987c4fab347784e602c00f17c5fc0bd7/tests/debug-example.sh and uncomment the line that does that automatically) you see how the lines get translated. We have a function that checks for each code chunk how indented it is so that it is de-dented in the otter buffer and the LSP requests and responses are translated accordingly when they contain ranges or positions. This idea was first introduced via a PR when a user wanted to use otter for neorg files and it worked quite nicely, but to be honest I didn't quite keep up with refactoring this to integrate it better, so the code might not be as nice. This is why I'm doing a bunch of other cleanups and type annotations in this branch as well.

Current issues:

Determining the indentation level can be fickel depending on if the treesitter injection query includes e.g. the preceding newline in the html example (that's why I'm removing some blank lines in this PR when determining the indentation)

Accepting a completion with blink.cmp puts it at the start of the linebor overwrites part of the word, even though I think I caught the respective position parameters in the otter.handler for completion resolve.

Plus we might want to experiment with just not changing indentations for languages that are not whitespace sensitive, this would make it a lot easier for most things that are not python or Haskell.

@jmbuhr

jmbuhr commented Feb 9, 2025

Copy link
Copy Markdown
Owner Author

Either way a second person taking a stab at this with fresh eyes is always appreciated! :)

@jmbuhr

jmbuhr commented Feb 9, 2025

Copy link
Copy Markdown
Owner Author

Ah, and the tabs vs. spaces thing was an instance where I noticed some autoformat of the html language server changing between tabs and spaces, which would confuse the otter buffer indentation check, not sure if that is still relevant.

@jmbuhr

jmbuhr commented Feb 9, 2025

Copy link
Copy Markdown
Owner Author

I also may have some debugging code in this PR related to investigating #197

@ctdunc

ctdunc commented Feb 26, 2025

Copy link
Copy Markdown

Hi there -- had some work travel come up, but I think I have made a bit of progress in debugging this issue, at least for my own understanding.

To me, it looks like the distinction is that in an indented block, after the first completion on any given line is resolved, no more calls to completionItem_resolve are made. The response in textDocument_completion contains all of the completions I would otherwise expect (for the raft buffer), but it looks like the next step of resolving user input/searching down is not being taken.

Still investigating why this is the case, but maybe this will prompt you in a productive direction as well 馃し

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.

2 participants