Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[html] HTML breadcrumbs don't work right when optional end tags omitted #89850

Open
dullroar opened this issue Feb 1, 2020 · 15 comments
Open
Assignees
Labels
feature-request Request for new features or functionality html HTML support issues
Milestone

Comments

@dullroar
Copy link

dullroar commented Feb 1, 2020

  • VSCode Version: 1.42.0-insider
  • OS Version: Linux Mint 19.3

Steps to Reproduce:

  1. Create an HTML document. Sprinkle liberally with <p>, <li> (inside of <ul> or <ol>, obviously), <tr> or <td> inside of <table>, and any other elements noted in the HTML5 spec as having optional end tags (see https://html.spec.whatwg.org/#syntax-tag-omission).
  2. If you closed the various tags above with their end tags (e.g., </p>), then run the page through something like HTML Tidy (I am using v5.7.28), passing in the --omit-optional-tags yes argument.
  3. Open the reformatted file in VSCode and navigate somewhere where there are multiple <p> or <li> or similar in a row. Note that there will be a breadcrumb for each open tag, because VSCode is apparently looking for end tags to close off the element, even though it is not required.

In a complex document, this make VSCode breadcrumbs pretty worthless. Here is an example, before a Tidy reformat, with focus on the third list item element in the fourth paragraph (note breadcrumbs working as expected):

BeforeReformat

And here are the breadcrumbs after the Tidy reformat, with focus on the same list item element:

AfterReformat

Does this issue occur when all extensions are disabled?: Yes

@jrieken jrieken assigned aeschli and unassigned jrieken Feb 3, 2020
@aeschli aeschli changed the title HTML breadcrumbs don't work right when optional end tags omitted [html] HTML breadcrumbs don't work right when optional end tags omitted Feb 4, 2020
@aeschli aeschli added html HTML support issues feature-request Request for new features or functionality labels Feb 4, 2020
@aeschli aeschli added this to the Backlog milestone Feb 4, 2020
@aeschli
Copy link
Contributor

aeschli commented Feb 4, 2020

Thanks for the pointer. The HTML language service is currently not aware of tags that can be omitted.

@dullroar
Copy link
Author

dullroar commented Feb 8, 2020

Thanks for making this a feature request. For the time being I have turned off breadcrumbs entirely, but for the one other person on the planet this probably affects, in case they find this through search, I went through all the breadcrumb settings and figured out either setting Symbol Path to last (which then shows only the highlighted element) or turning off Show Fields "fixed" the problem, but with possibly other issues in other file types. I decided to reclaim a line of vertical line space for now by simply switching them off completely.

@nikeee
Copy link
Contributor

nikeee commented Apr 3, 2020

It does also affect code formatting (see #94287). When formatting this:

<p>Lorem
<p>ipsum
<p>dolor
<p>sit
<p>amet

you get:

<p>Lorem
    <p>ipsum
        <p>dolor
            <p>sit
                <p>amet

@aeschli
Copy link
Contributor

aeschli commented Apr 3, 2020

@nikeee We use beautifyjs as the underlying HTML formatter. Would you mind filing an issue at https://github.com/beautify-web/js-beautify as well?

@nikeee
Copy link
Contributor

nikeee commented Apr 3, 2020

@aeschli: There already is an issue regarding this: beautifier/js-beautify#1503

@aeschli
Copy link
Contributor

aeschli commented Apr 3, 2020

Thanks for the link, sorry, I didn't check myself.

@bitwiseman
Copy link

bitwiseman commented Apr 5, 2020

v1.11.0 of js-beautify fixes the formatting issue that @nikeee commented on. This doesn't fix the breadcrumb issue.

@dullroar
Copy link
Author

dullroar commented Apr 5, 2020

@bitwiseman I presume you are talking about the js-beautify pretty print issue, because for Code itself, at v 1.44.0-insider the original problem with breadcrumbs is still there.

@bitwiseman
Copy link

Updated.

@bitwiseman
Copy link

@dullroar @nikeee
Could you try your input files on https://beautifier.io ? To make sure the fix works as you expect?

@dullroar
Copy link
Author

dullroar commented Apr 6, 2020

@bitwiseman It does, as far as beautifying goes, although that isn't the original bug in VS Code I filed, which was that breadcrumbs are broken. Or is this an essential first step in fixing that?

@bitwiseman
Copy link

@dullroar
Oh, I see what happened. @nikeee reported the formatting issue as a comment on this bug (even though they are completely separate. :( I've updated my comment above (again). Thanks for checking on the formatting.

@aeschli
I have a pretty good understanding of the optional closing tags now. I might be able to implement them for https://github.com/microsoft/vscode-html-languageservice/ . I'm assuming the change would go somewhere around here:
https://github.com/microsoft/vscode-html-languageservice/blob/f4777d03d92a79e70d09d3c44e1a835cc356a9f4/src/parser/htmlParser.ts#L79

Right?

@nikeee
Copy link
Contributor

nikeee commented Apr 7, 2020

even though they are completely separate

Well, I originally opened a separate issue (#94287) regarding the breadcrumbs and the formatting issue (I assumed that both had the same origin in the code). As it was closed as a duplicate, I put the formatting part into this issue as well. It turned out that they had different origins.

I was thinking too about taking a look into the HTML service, but I lack the time ATM.

@dullroar
Copy link
Author

Any progress on this? I like having the breadcrumbs turned on in general, but this issue makes them less than useful for me in HTML.

@aeschli
Copy link
Contributor

aeschli commented Jan 5, 2021

Related issue in microsoft/vscode-html-languageservice#63

@bitwiseman Sorry for only see your question now. Yes, that needs to go into the HTML parser. If you want to give it a try, that would be fantastic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality html HTML support issues
Projects
None yet
Development

No branches or pull requests

5 participants