Repository navigation
invalid/non stable ast for combinator with comment #189
Description
Input:
h2 /*test*/ h4 { } /* Here we don't have `comment` node in ast (store comment in `raws`) */
h2/*test*/h4 { } /* Comment in ast */It is do impossible analyzes comments, also adopt new version to stylelint and prettier. For me it is bug, but fixing this change ast.
Activity
alexander-akait commentedon Mar 15, 2019
/cc @ai what do you think about this
ai commentedon Mar 15, 2019
Why it will change AST?
alexander-akait commentedon Mar 16, 2019
@ai new
commentnode appears in ast(before it was inraws), just want to clarify should we release this as patch or major?ai commentedon Mar 16, 2019
Technically it should be major
alexander-akait commentedon Mar 19, 2019
Same problem for
[ /*t*/ title /*t*/ = /*t*/ "Something" /*t*/ ], comments should be part of ast notrawalexander-akait commentedon Mar 19, 2019
/cc @ai i ahve some questions about spaces too
Example:
What node(s) should store spaces in this case (inside
raws?)? Because now logic in parser is very misleading and I can't understand whether we do it right or not. We haveattribute,value,insensitivevalues.ai commentedon Mar 19, 2019
To be honest, I am not sure that I know the best answer.
But I think in selector parser we should work with spaces in a different way, compare to PostCSS core. In main CSS syntax, whitespaces don’t mean anything. In selector, whitespace can have a lot of meanings.
So, I think it could be better to have a special token for spaces.
alexander-akait commentedon Mar 19, 2019
@ai yep, we already have special token for
space, my questions about where we should store meta information about space (raws). Let's see on example above:raws.beforeorraws.attribute.before?raws.attribute.beforeorraws.operatore.before?raws.operator.afterorraws.value.beforeraws.value.afterorraws.insensitive.before(insensitivewill be renamed inflagin next major)raws.insensitive.afterorraws.after(insensitivewill be renamed inflagin next major)ai commentedon Mar 19, 2019
Why do we need a
rawsfor whitespaces when we havespacetoken?alexander-akait commentedon Mar 19, 2019
@ai hm can you provide example/PoC of ast with
spacetoken (based on example above)? We need this spaces for linting/printing instylelintandprettierai commentedon Mar 19, 2019
a b>{ type: "word", value: "a" }, { type: "space", value: " " }, { type: "word", value: "b" }alexander-akait commentedon Mar 19, 2019
@ai in example above
spaceisdescendant selector, we already providecombinatoras separate node, but in my example spaces mean nothing so we omit them from astai commentedon Mar 19, 2019
I am not sure that we should omit them from AST.
alexander-akait commentedon Mar 19, 2019
@ai I do not see their meaning as they really do not carry here any semantic load, also we do this right now, we can plan this on next major, but will be great solve problems with spaces using
rawsfor next releaseai commentedon Mar 19, 2019
Yeap. This discussion is on you. Sorry, that I can't help right now.
alexander-akait commentedon Mar 20, 2019
@ai Maybe not related to issue, but what is blocker integrate selector parser in postcss? Non standard syntax? Or nobody send a PR?
ai commentedon Mar 20, 2019
@evilebottnawi I am afraid that we will freeze API which will not be the best.
alexander-akait commentedon Mar 21, 2019
@ai we can afraid this forever 😄 To be honestly ast of
postcssis not enough for prettier/stylelint nowadays (prettier breaks code in many cases and i recommended don't use this for css/scss/less/etc). I think we should start integrationselector/value/mediaparsers otherwise we will need do fork postcss and continue development 😞alexander-akait commentedon Mar 21, 2019
Also we can release this under flag/option and as experimental (adding information to readme about non stable)
We have big tests code base in stylelint and prettier and webpack so i think we catch all problems very fast
ai commentedon Mar 21, 2019
RIght now our main focus is visitor API postcss/postcss#1245
alexander-akait commentedon Mar 21, 2019
Great feature 👍 Already look on this.
@ai Is there any sense send a PR or better fork postcss and starting own development for prettier? Because i don't want to waste my time on something what will be never merged.
ai commentedon Mar 21, 2019
@evilebottnawi I have a better plan. Let’s release new AST in
postcss-selector-parserand if nobody will complain about it, we can move it to PostCSSalexander-akait commentedon Mar 21, 2019
@ai okay 👍 i will ping you before release or when i have some questions