helix icon indicating copy to clipboard operation
helix copied to clipboard

XML highlighting fails with comment of certain length in document root

Open LeoniePhiline opened this issue 2 years ago • 6 comments

Summary

XML highlighting fails if there is a comment in the XML document root which has at least a certain length.

This fails:

<?xml version="1.0" encoding="utf-8"?>
<!-- XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX -->
<svg version="1.1" xmlns="http://www.w3.org/2000/svg">
</svg>

image

This works (One "X" less in the comment line):

<?xml version="1.0" encoding="utf-8"?>
<!-- XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX -->
<svg version="1.1" xmlns="http://www.w3.org/2000/svg">
</svg>

image

Reproduction Steps

I tried this:

  1. hx
  2. Paste "This fails" XML content
  3. Set :lang xml
  4. Observe failed highlighting
  5. Remove one X from the comment line
  6. Observe successful highlighting

I expected this to happen:

Highlighting succeeds

Instead, this happened:

Highlighting fails (see screenshot)

Helix log

(Nothing in the helix log.)

Platform

openSUSE Tumbleweed - Linux

Terminal Emulator

Konsole

Helix Version

helix 22.12

LeoniePhiline avatar Jan 09 '23 17:01 LeoniePhiline

This is a bug in https://github.com/RenjiSann/tree-sitter-xml. Putting the comment near the top of the file seems to cause problems. @RenjiSann, can you take a look at this when you have time?

Ordoviz avatar Jan 09 '23 18:01 Ordoviz

Thank you for letting me know about this. I will try to investigate

RenjiSann avatar Jan 09 '23 19:01 RenjiSann

The error is already present in the project I forked, I only modified it slightly and added a default highlight.scm. I will try to dive in the XML syntax and find the error, but I'm not sure I will succeed.

RenjiSann avatar Jan 09 '23 20:01 RenjiSann

I've tried to understand how's the grammar working, but tbh it's to big for me to track the bug down any time soon. I might do it all over again, though it will be much simpler, just to ensure syntax highlighting

RenjiSann avatar Jan 14 '23 23:01 RenjiSann

Maybe the author of the original repository will find a bug? Can you take a look @dorgnarg?

ghost avatar Jan 15 '23 09:01 ghost

Hi ! I was able to take a look at the bug. It appeared that the project I forked confused single and multiple spaces in the grammar. This is not entirely fixed yet because I am trying to avoid regression and lack proper testing. Anyway, release 0.0.2 should fix this particular issue. Please keep me informed of any other bugs you find, that will help me to test more thoroughly.

RenjiSann avatar Jan 15 '23 13:01 RenjiSann