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

Bring HTML completions to the year 2015 🚀 #16

Merged
merged 1 commit into from
Jun 20, 2015
Merged

Bring HTML completions to the year 2015 🚀 #16

merged 1 commit into from
Jun 20, 2015

Conversation

kleinfreund
Copy link
Contributor

Changes made to html_completions.py

Changes made to the file are mainly based upon the HTML element reference from the Mozilla Developer Network. I did not make changes to tags and/or attributes that are marked as obsolete (I did neither remove any obsolete attributes nor add missing ones).

I tested whether the completion works for all tags inside normal_tags and it does.

  • Adding new/missing tags: main, content, data, datalist, details, element, embed, output, picture (:rocket:), progress, rp, rt, rtc, ruby, shadow, summary and video
  • Adding tags to get_tag_to_attributes map (so they get the global attributes)
  • Adding HTML5 attributes (download, media, ping, etc.) if applicable
  • Added special-cased completions for audio, progress, source, track and video
  • consistent use of quotes (e.g. '' only, escaped quotes \" were not touched as they’re used as actual output)
  • Replacing if with elif in get_attribute_completions, because the conditions are exclusive

Stats:

Original File New File
LoC 370 485
Chars 24864 20809

Internal tests fail

Note that I tried to run the internal test cases within that file by executing the following:

import HTML.html_completions
HTML.html_completions.Unittest.run()

The tests fail with this:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "C:\Users\Philipp\dev\Sublime Text Build 3092 x64\Data\Packages\HTML\html_completions.py", line 420, in run
    test.test_simple_completion()
  File "C:\Users\Philipp\dev\Sublime Text Build 3092 x64\Data\Packages\HTML\html_completions.py", line 441, in test_simple_completion
    self.assertEqual(flags, sublime.INHIBIT_WORD_COMPLETIONS | sublime.INHIBIT_EXPLICIT_COMPLETIONS)
  File "./unittest/case.py", line 641, in assertEqual
  File "./unittest/case.py", line 634, in _baseAssertEqual
AssertionError: 0 != 3
reloading plugin HTML.html_completions

This does not look related to my changes. Running the same from within my installed version that does not have the changes results in the same.


Have a good day and let me know if there's something I need to adjust.

('source\tTag', 'source src=\"$1\" type=\"$2\">'),
('style\tTag', 'style type=\"${1:text/css}\">$0</style>'),
('track\tTag', 'track kind=\"$1\" src=\"$2\">'),
('video\tTag', 'video src=\"$1\">$0</video>')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just want to mention that these completions are inconsistent, which was not in the scope of your changes, but anyway. Some tags are closed and some are not. They should end with /> and have a $0 anchor.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The completions for the link and img tag omit the closing tag and were there before. They don’t end in /> so I tried to stick with that.

Do you want me to add closing slashes to void tags like <img src="" />?

Also as far as I can tell, the $0 is only used when tags are actually closed and the last tabbing position should be inside the tag pair. However track, source and progress omit the end tag as well.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ending self-closing tags with /> sort of died when HTML5 came out as it is valid to to just do <img src="...">

Some devs are still in the self close habbit, but its not necessary.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I admit that I don't know enough about the specifications of all the new HTML5 tags and what you can actually place in them (it seems weird for me to have content for a video/audio tag for example), but I do know that not closing XML tags fucks with my mind (justifiably) and is imo a bad habbit.

The $0 anchor would just be for "continue typing here, once all parameters have been filled" - and this is missing for img, link, param, progress, source and track.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah - it was a standard back when we validated our sites for be HTML 4.1 Strict or whatever - totally not necessary now. Feels good :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The $0 anchor would just be for "continue typing here, once all parameters have been filled"

@FichteFoll As far as I observed it, having the last caret position after the complete tag is baked in somewhere. What you want is type img, hit tab, have the caret after src=", hit tab again and have the caret after <img src="">, right?

If so, it's already working like that without the need to explicitly add $0.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kleinfreund Oh yeah, it is. Sorry, my bad. I should have known this. 😰 (Or, actually, I should have tested it since the thought did cross my mind.)

Opinions on self-closing tags?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion, the time has long come that tags like img – as @wesbos already said – should not be marked as self-closing by adding the slash. There is no longer a necessity for it and no one stops you if you have that habit. Boils down to adding one/two less characters. Not a big deal.

@wesbos
Copy link

wesbos commented Jun 19, 2015

I can't speak of the Python code, but the logic behind the HTML5 standard tags is sound, so 👍

sublimehq pushed a commit that referenced this pull request Jun 20, 2015
Bring HTML completions to the year 2015 🚀
@sublimehq sublimehq merged commit 5086874 into sublimehq:master Jun 20, 2015
varungandhi-src referenced this pull request in sourcegraph/Packages Jan 11, 2022
Co-authored-by: Stephen Gutekanst <stephen.gutekanst@gmail.com>
deathaxe pushed a commit to deathaxe/sublime-packages that referenced this pull request Jun 7, 2022
gitworkflows pushed a commit to gsoc2/Packages that referenced this pull request Apr 8, 2024
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.

None yet

4 participants