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

Add processing to the valid user-defined variables #1259

Merged
merged 4 commits into from May 4, 2019

Conversation

Projects
None yet
2 participants
@isolovev
Copy link
Member

commented May 1, 2019

Resolve #1255

@isolovev isolovev changed the title Resolve #1255 Add processing to the valid user-defined variables May 1, 2019

@isolovev

This comment has been minimized.

Copy link
Member Author

commented May 1, 2019

@ai Не могу понять почему упали тесты 😢

@ai

This comment has been minimized.

Copy link
Member

commented May 1, 2019

Они и должны упасть, так как ты добавил тесты в этом PR а парсер исправил в PR в другой репозиторий

@@ -154,7 +162,7 @@ export default class Parser {
this.current = node
}

decl (tokens) {
decl (tokens, customProperty) {

This comment has been minimized.

Copy link
@ai

ai May 2, 2019

Member

А сделай лучше как константу в начале файле. Тогда не надо будет передавать её в метод.

@@ -92,6 +92,7 @@ export default class Parser {
let colon = false
let bracket = null
let brackets = []
let customProperty = /^--/.test(start[1])

This comment has been minimized.

Copy link
@ai

ai May 2, 2019

Member

регэксп не самый лучший способ проверку. лучше делать prop.slice(0, 2) === '--'

@ai

This comment has been minimized.

Copy link
Member

commented May 2, 2019

А нет, тут тесты падать не должны. Видимо PR работает не как планировалось и что-то ломает.

@isolovev

This comment has been minimized.

Copy link
Member Author

commented May 2, 2019

Понял. Буду проверять!

@isolovev

This comment has been minimized.

Copy link
Member Author

commented May 2, 2019

Нужно у TravisCI поток node перезапустить

@isolovev

This comment has been minimized.

Copy link
Member Author

commented May 2, 2019

В ноде 12.1.0 падает. Правлю

@ai

This comment has been minimized.

Copy link
Member

commented May 2, 2019

Не понял (у CI поток node каждой раз запускается сразу).

Может ты какой-то кеш имеешь в виду? (Например, кеш yarn плохо работает при зависимостях из git).

@isolovev

This comment has been minimized.

Copy link
Member Author

commented May 2, 2019

Он упал с версией ноды 12.1.0, а с 6, 8, 10 не падал

@isolovev

This comment has been minimized.

Copy link
Member Author

commented May 2, 2019

Я из зависимостей ничего не обновлял

@ai

This comment has been minimized.

Copy link
Member

commented May 2, 2019

Да, у JSDoc есть проблема с последней нодой. Замени в .travis.yml node на "11".

Я потом заменю JSDoc на Documentation.js.

@isolovev

This comment has been minimized.

Copy link
Member Author

commented May 2, 2019

Ура, тесты прошли :)

@isolovev

This comment has been minimized.

Copy link
Member Author

commented May 3, 2019

Тут ещё что-то нужно править?

@ai

This comment has been minimized.

Copy link
Member

commented May 3, 2019

Вроде всё норм. Попробую на днях найти время для более глубокого код-ревью и релиза.

@ai ai merged commit 3a69eea into postcss:master May 4, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ai

This comment has been minimized.

Copy link
Member

commented May 4, 2019

Released in 7.0.15.

} else if (type === '{' && this.customProperty) {
token = this.tokenizer.nextToken()

if (/\n/.test(token[1])) {

This comment has been minimized.

Copy link
@ai

ai May 4, 2019

Member

@isolovev Нельзя так полагаться на \n. В CSS \n не влияет не на что.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.