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

Tags generated for SCSS include characters that prevent completion #3409

Closed
Roy-Orbison opened this issue Jun 10, 2022 · 5 comments · Fixed by #3410
Closed

Tags generated for SCSS include characters that prevent completion #3409

Roy-Orbison opened this issue Jun 10, 2022 · 5 comments · Fixed by #3410
Assignees

Comments

@Roy-Orbison
Copy link
Contributor

In my original, the punctuation used to denote classes is excluded from the tag itself. In @masatake's revised version, the % and . are included, which breaks autocompletion in Vim because those characters are not normally part of keywords. One can add them with isk+=%,. but this breaks chaining classes as %foo.bar is then seen by Vim as a single keyword where humans and uctags do not.

Also, the insertion of : into the keyword characters of c, P & i cause a similar issue to the above, and results in tags like .foo:not. These are a bit unexpected because pseudo-classes have language-level completion support.

I don't want to create a PR to undo those changes without knowing why they were made, so could someone please clarify those decisions?

Applying this patch, my tags work as expected with only isk+=- in my .vimrc.

--- a/optlib/scss.ctags
+++ b/optlib/scss.ctags
@@ -59,9 +59,9 @@
 --_mtable-regex-SCSS=toplevel/:[^\n{]+[;{]\n?//
 --_mtable-regex-SCSS=toplevel/\$([A-Za-z0-9_-]+)[ \t]*:[ \t]*\(/\1/v/{tenter=map}
 --_mtable-regex-SCSS=toplevel/\$([A-Za-z0-9_-]+)[ \t]*:[^\n]*\n?/\1/v/
---_mtable-regex-SCSS=toplevel/([.][A-Za-z0-9_:-]+)/\1/c/
---_mtable-regex-SCSS=toplevel/(%[A-Za-z0-9_:-]+)/\1/P/
---_mtable-regex-SCSS=toplevel/#([A-Za-z0-9_:-]+)/\1/i/
+--_mtable-regex-SCSS=toplevel/[.]([A-Za-z0-9_-]+)/\1/c/
+--_mtable-regex-SCSS=toplevel/[%]([A-Za-z0-9_-]+)/\1/P/
+--_mtable-regex-SCSS=toplevel/#([A-Za-z0-9_-]+)/\1/i/
 --_mtable-regex-SCSS=toplevel/.//
 --_mtable-regex-SCSS=comment/\*\///{tleave}
 --_mtable-regex-SCSS=comment/.//
@masatake masatake self-assigned this Jun 14, 2022
@masatake
Copy link
Member

I found my comment about including the characters ([.%]) to tag names:
#801 (comment)

I included the characters because the CSS parser included the characters.

@masatake
Copy link
Member

For the CSS parser, I think including the characters looks natural.
However, after reading test cases, for the SCSS parser, I DON'T think including the characters looks natural.

[jet@living]~/var/util-linux% ~/bin/ctags --list-kinds=SCSS
~/bin/ctags --list-kinds=SCSS
m  mixins
f  functions
v  variables
c  classes
P  placeholder classes
i  identities
z  function parameters
[jet@living]~/var/util-linux% ~/bin/ctags --list-kinds=CSS
~/bin/ctags --list-kinds=CSS
c  classes
s  selectors
i  identities

The CSS parser has selector kind. The rest of the selectors and identities may be recorded as selectors.

input.css:

ul li { padding-left: 1em; }
.foo a, .foo b { color: blue; }

css.tags:

ul li	input.css	/^ul li { padding-left: 1em; }$/;"	kind:selector	line:22	language:CSS	roles:def
.foo a	input.css	/^.foo a, .foo b { color: blue; }$/;"	kind:selector	line:23	language:CSS	roles:def
.foo b	input.css	/^.foo a, .foo b { color: blue; }$/;"	kind:selector	line:23	language:CSS	roles:def

If we remove the characters from the tags having selector kind, understanding tags output becomes hard.

The SCSS parser doesn't record selectors.

input.scss:

.alert, .warning {
  ul, p {
    margin-right: 0;
    margin-left: 0;
    padding-bottom: 0;
  }
}

scss.tags:

.alert	input.scss	/^.alert, .warning {$/;"	kind:class	line:2	language:SCSS	roles:def
.warning	input.scss	/^.alert, .warning {$/;"	kind:class	line:2	language:SCSS	roles:def

The kind names give users enough information about the tag entries.
So NOW I think the prefix characters in SCSS are not needed.

Keeping the SCSS parser and the CSS parser in the meaning of "kind names" is nice but optional.

Supporting comfortable operation in vim is nice but optional. universal-ctags should be editor-neutral.

Do you think removing the prefix characters is better even if you don't use vim?
In the other words, do you think removing them makes the SCSS parser more consistent and/or better?
If the answer is yes, please make a pull request:-)

As I wrote in this comment, in my limited CSS and SCSS knowledge, the answer is yes.

Thank you for your deliberate approach. So I got a chance to tell you my idea.

@masatake
Copy link
Member

...but... when the SCSS parser supports filling scope fields and fully-qualified tags, I guess we may need the prefix characters.

@Roy-Orbison
Copy link
Contributor Author

For the CSS parser, I think including the characters looks natural. However, after reading test cases, for the SCSS parser, I DON'T think including the characters looks natural.

For the record, PHP tags do not include the sigil ($) for variables, and they complete just fine. I'm not sure what other languages would need punctuation characters in keywords.

The CSS parser has selector kind. The rest of the selectors and identities may be recorded as selectors.
...

.foo b	input.css	/^.foo a, .foo b { color: blue; }$/;"	kind:selector	line:23	language:CSS	roles:def

The only use I can see for having an entire selector in the tag is for locating it in the source file. I can't imagine it would aid authoring CSS. I'd like to know what use people get from that kind, because it seems that would make it hard to find tags that don't begin with an exact search string.

If we remove the characters from the tags having selector kind, understanding tags output becomes hard.

I don't think it makes it particularly hard because the pattern it's matching is right there, so you can see what the keyword is, even without the kind: field.

Supporting comfortable operation in vim is nice but optional. universal-ctags should be editor-neutral.

Yes, I just figured adding the punctuation was a break in compatibility because updating uctags broke completion for me.

Do you think removing the prefix characters is better even if you don't use vim? In the other words, do you think removing them makes the SCSS parser more consistent and/or better? If the answer is yes, please make a pull request:-)

As I wrote in this comment, in my limited CSS and SCSS knowledge, the answer is yes.

I think yes because the punctuation is really just a delimiter, in .foo, the class name is just foo which is what you'd put in your HTML, etc. This also makes it possible to use CSS/SCSS complete class names in other file types.

@Roy-Orbison
Copy link
Contributor Author

Do you mean fully qualified names, like list.append()? They more of a language-level thing to me, and should probably be individual tags.

Roy-Orbison added a commit to Roy-Orbison/ctags that referenced this issue Jun 17, 2022
Prevent appended pseudos as well. This should make the tags file output
more consistent with other language parsers, where the identifiers are
more important than the delimiters that denote their types. This aids
completion in existing tools and across languages (like HTML) where the
delimiters are not used.

Closes universal-ctags#3409
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 a pull request may close this issue.

2 participants