CSS autocomplete loses items

[m1ga]

Thanks in advance for your bug report!

• [X] Have you reproduced issue in safe mode?
• [X] Have you used the debugging guide to try to resolve the issue?
• [X] Have you checked our FAQs to make sure your question isn't answered there?
• [X] Have you checked to make sure your issue does not already exist?
• [X] Have you checked you are on the latest release of Pulsar?

What happened?

While typing in a CSS file to add justify-content it will show the elements only if I type ju. As soon as I type the next letters it won't show it:

https://github.com/pulsar-edit/pulsar/assets/4334997/1a7b1ea8-0686-4b70-a459-226e0e7d1937

Pulsar version

1.107.1

Which OS does this happen on?

🐧 Red Hat based (Fedora, Alma, RockyLinux, CentOS Stream, etc.)

OS details

Fedora 38

Which CPU architecture are you running this on?

64-bit(x86_64)

What steps are needed to reproduce this?

• set file to CSS
• type ju and it will show justifty-*
• as soon as you type jus it will remove all justify-* elements

Additional Information:

No response

[confused-Techie]

Thanks a ton for reporting this issue!

One thing that's vital to know, is which grammar are you using? You said you set it to a CSS grammar, but is this TextMate, Node Tree-Sitter, or the new experimental Tree-Sitter?

[m1ga]

the default when you open a CSS file. I didn't switch on "use modern tree-sitter". So just language-css and autocomplete-css.

[savetheclocktower]

I can't reproduce this no matter which of the CSS grammars I use. All of the justify-* options remain available after I type the s in jus.

@m1ga, can you do something just for diagnostic purposes? Go into your installed packages, search on autocomplete, and disable everything except autocomplete-css and autocomplete-plus. Then either relaunch Pulsar or run the Window: Reload command. I'm curious whether some other provider is asserting itself.

[m1ga]

It looks like it depends on the size of my file:

https://github.com/pulsar-edit/pulsar/assets/4334997/ff56a698-c193-43fe-be62-565a3d96716b

.sportform {
  margin: auto;
  display: block;
  max-width: 800px;
  width: 100%;
}

.sportform .pages {
  text-align: center;
}

.sportform input[type=radio] {
  display: none;
}

div {

}
.sportform input[type=radio]:checked~img {
  border: 4px solid red;
}

@media (max-width: 768px){
  .sportform .box img {
    width: 100px;
    height: 100px;
  }
}
@media (max-width: 512px){
  .sportform .box img {
    width: 60px;
    height: 60px;
    padding: 10px;
  }
}

doesn't work. But if I remove everything below the div it will work

[savetheclocktower]

OK, that screencast makes it clearer to me. Presumably you're using a legacy Tree-sitter grammar, since that's the default. I can reproduce that outcome on both the legacy Tree-sitter CSS grammar and the modern Tree-sitter CSS grammar.

The core issue there is that Tree-sitter is making its best guess at the meaning of what you're typing. It makes no attempt to guess what kind of thing you're typing until you type the y in justify, at which point it changes its mind and decides that justify is a tag name, and you're typing a selector. Amazingly, it interprets justify as belonging to the .sportform input[type=radio]:checked~img selector, even though there's an intervening }. That's why this only happens in the middle of the file — if you were at the end of the file, it'd have no subsequent selectors to try to join to.

What's even more bizarre is that this behavior only asserts itself once the property you're typing reaches seven letters. There's nothing magical about justify; it could've been abcdefg. I bet this is also the cause of what you initially reported, because in your first screencast I now see that the color of ju changes when it becomes jus. So there must be some sort of length heuristic that Tree-sitter uses, one that depends on the length of the subsequent selector.

Needless to say, this is not smart behavior on Tree-sitter's part, or even intended behavior. I filed this issue with tree-sitter-css last week precisely because it affects our ability to deliver accurate contextual completions while the user is typing.

The quickest workaround would be to force Pulsar to use the TextMate-style grammar. You can put this at the bottom of your config file:

".css.source":
  core:
    useTreeSitterParsers: false

For this to get better otherwise, someone will have to fix tree-sitter-css, and if I'm unlucky, it'll fall to me.

[savetheclocktower]

@confused-Techie, I'm not sure what to do with this. Now that I think about it, there are two ways this can be fixed:

1. We wait for an improvement in tree-sitter-css.
2. We give autocomplete-css some amount of Tree-sitter awareness instead of just inspecting the scope names it produces, thereby allowing us to ignore its parsing choices when we know they're silly.

So I suppose we should leave this open until we know what the answer is. I'll assign it to myself. I don't think this has a quick fix, but I don't want it to fall off the radar.

[m1ga]

Thank for the detailed explanation! Yes, it works fine at the bottom of the file and useTreeSitterParsers: false works fine too! I have the feeling that it worked better before even without a change. But for now the new setting is fine for me as long as it works better :smile:

[savetheclocktower]

What's weird is that I didn't think autocomplete-css worked at all for the legacy Tree-sitter grammar until #601, but maybe I was mistaken about that.

[savetheclocktower]

Good god, this is the weirdest I've ever seen a Tree-sitter parser act. The number of characters typed before Tree-sitter changes its mind seems to be entirely dependent on how much parse-able text exists subsequent to the block. If I add more stuff to your sample CSS file, the threshold decreases; if I comment out all but the next selector, the threshold increases to as many as 19 characters.

[confused-Techie]

@savetheclocktower This is a fantastic write up of what exactly is going on here, and I seriously appreciate it. Makes sense even to me not spending much time with Tree Sitter grammars.

But as for what the right answer is, I'll be honest, I'm looking at the official TreeSitter CSS Repo and seeing issues opened in 2019 that haven't received a single comment, so it doesn't make me super hopeful it'll be fixed in any way fast on their end.

So I think the right answer for us, is to either implement a fix on our end and PR to the repo, or make autocomplete-css more Tree Sitter aware to complete better.

---

An aside, considering the amount of trouble we have been having with autocomplete-css I almost wonder if it's worth it to rethink how we provide completions. Since over and over we are seeing issues with it, I wonder what kind of performance hit we would expect if autocomplete-css just ran it's own tooling to generate an AST of the CSS and provide completions based off that. I realize it may not be the best solution, but it may be our most consistent solution.

Or really find any way to provide completions that are not 100% reliant on perfect parsing by whatever grammar is in use.

[savetheclocktower]

Since over and over we are seeing issues with it, I wonder what kind of performance hit we would expect if autocomplete-css just ran it's own tooling to generate an AST of the CSS and provide completions based off that.

Yeah, that's my feeling. I think that you could get a very accurate picture of what to suggest just by looking at the current line, as long as you also knew if you were inside a block or not. I'm imagining some sort of low-dependency NPM package that could help with that — or, failing that, even a poor-man’s parser that just ran a few regexes against a line to understand it.

So that's on my medium-term list.

I played around with tree-sitter-css and I'm able to rewrite the grammar so that it always prefers property_name when a declaration is incomplete, but it's a bit of a hack. I doubt it'd be accepted as a PR, and I don't want us to maintain a fork indefinitely. The right answer is for the parser to represent invalid states in the tree more usefully, but that might have to wait on this issue.

[confused-Techie]

@savetheclocktower I think it's a fantastic idea to find out if you are in a block or not and suggest on that. I've mentioned before, but one day I love the concept of being able to autocomplete based entirely on correct syntax, but that would be something saved fully for providing completions on each value itself, so guess it shouldn't be to relevant here.

But as for the idea of a small package to handle this, something I'd be happy to rig up, or at least suggest, is that autocomplete-css could just backtrack from the current buffer point until it finds one of the following characters first:

• {: If this is found and } is never encountered, cool we are in a block, autocomplete with that assumption
• :: If this is found and no \n or ; is found, then we can assume we are autocompleting the value of any given key.

Beyond that, we could check for more, but just backtracking through every point with those assumptions in mind may be enough for our purposes, and could be a simple function, that self calls for recursion, passing state of if it's found any of those characters.

But glad we seem to agree, that a less grammar aware approach may be best in the long term

[confused-Techie]

Also just wanting to add here @savetheclocktower if my explanation of a simple loop didn't make sense, I've written up a draft of what I'd consider that could work here.

The following loop, provided the snippet of text from where the current buffer is, back to the beginning of the file, can reliably determine if you are within a block of code, if you are completing a value of code, or it doesn't know, in which case we can assume we are outside of a block.

function loop(text, idx, finds) {
  idx ??= text.length-1;

  let char = text.charAt(idx);

  if (char === "\n") {
    finds.newline = true;
  } else if (char === ";") {
    finds.semicolon = true;
  } else if (char === "}") {
    finds.close_bracket = true;
  } else if (char === ":") {
    if (!finds.newline && !finds.semicolon) {
      return "value";
    }
  } else if (char === "{") {
    if (!finds.close_bracket) {
      return "block";
    }
  }

  if (idx === 0) {
    return "unknown";
  } else {
    return loop(text, idx-1, finds);
  }
}

So using this, we could run this function on any text provided and if:

• unknown: Completing for the selector
• value: Completing for any valid value
• block: Completing for the key

Of course I'm sure there are ways to optimize this, or could let these values return what you are actually completing, was just a quick mockup while I had it in my head, if this was the route we wanted to go, and no matter what is hopefully faster than generating a AST, and much more specific of a use case then I'd be willing to bet any found NPM module would be.

And to call within autocomplete-css just use loop(textFromBuffer, null, {});

[savetheclocktower]

Yeah, wrote up some notes of my own :)

• From cursor position, scan backwards for nearest {, then forwards for nearest }
• If cursor is inside a block…
  • Is the cursor before the colon? Is there no colon on the line at all?
    • This is a property completion
    • Are nested selectors valid? (SASS, LESS, and soon CSS)
      • This is also a selector completion (add suggestions to property completion)
  • Is the cursor after the colon?
    • This is a value completion
• If cursor is not inside a block…
  • This is a selector completion

And we'd need to have some special cases for colons in pseudoselectors, and perhaps to avoid matching { or } in an attribute value, but this'd be a good start.

[confused-Techie]

Ahh, yeah good notes there, much more specific than what I drafted. But yeah even that logic is a great start, and would hopefully get this working in 90% of uses, rather than what we have now

[pitva90]

I'm currently using the rolling release 1.113.2024020917

Is it possible that if I put that code at the end of my config file, Pulsar will replace the config.cson with the new one after reboot? So will I lose all my settings?

[savetheclocktower]

Is it possible that if I put that code at the end of my config file, Pulsar will replace the config.cson with the new one after reboot? So will I lose all my settings?

…no? I'm not sure what you mean by “the new one” there — there's just one config.cson. When you make changes in the settings UI, they propagate to the config.cson. If there's an error in your config.cson, Pulsar won't just toss it out and start over; it knows how important your config file is.

All the same, though: if you're worried about losing your settings, you can back up your config.cson, or even all of your .pulsar folder, before you make a change.

[pitva90]

That code didn't work for me when I was on regular version 1.113. It kept losing items in autocomplete. Exactly as seen in the video. But I didn't want to bother here, so I tried a new rolling release if it's not something that's already fixed. And now when I put that code at the end, I always start with a fresh new config file after restarting the Pulsar. I tried twice.

[confused-Techie]

@pitva90 By "that code" I'm assuming you mean:

".css.source":
  core:
    useTreeSitterParsers: false

Since the code I posted won't do anything in the config.cson. But otherwise, if Pulsar is losing your config on reboot that's an entirely different issue we would need to investigate, as that should never happen under any circumstances and has never been documented before. If simply restarting Pulsar is rewriting your config, would you mind creating an issue so we can investigate that specifically?