Bracketed lists #2279
Merged
Bracketed lists #2279
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
xzyfer
added a commit
to xzyfer/sass-spec
that referenced
this pull request
Dec 31, 2016
xzyfer
force-pushed
the
bracked-lists
branch
from
e823613 to
59c9a91
Compare
This is the idea: in the same way the old parser looks for parenthesis delimiters, let's add another case for brackets delimiters. In case an open bracket is found, the parser will look for a list. There are three possible results of parsing a list: First, the parsed expression is not a list at all. libsass parser tends to avoid wrapping if lists are only one item lists. But bracketed lists prevent unwrapping so the item becomes the solely item for a new bracketed list. Example: singleton-item --> [ singleton-item ] Second, the list is indeed a list with some explicit delimiter. This case is similar to the previous one. Brackets prevent unwrapping so this time is the list as a whole which become the unique item of the bracketed list. Example: (item1, item2, ...) --> [ (item1, item2, ...) ] [item1, item2, ...] --> [ [item1, item2, ...] ] Third, the list is a list with no delimiters. So its items actually belong to the bracketed list. The parse will fix the delimiter of the list for them to be brackets. Example: Suppose | ... | stands for a list with no delimiters. |item1, item2, ...| --> [item1, item2, ...]
Functions operating on a copy of lists must copy the delimiter also.
First, instead of directly writting the left or right parenthesis characters, now functions are called for left or right delimiters and change in case of parenthesis or brackets. Second, in case of bracketed list, always add the brackets. Also add comma in case it is a one-item comma-separate list. Otherwise it would become a one-item space-separated list. Third, libsass prevent from emitting invisible items. Thus, an empty space / comma separated list won't be printed at all but bracketed list are never invisible, event the empty ones.
mgreter
reviewed
Jan 2, 2017
| @@ -35,6 +35,13 @@ enum Sass_Separator { | |||
| SASS_HASH | |||
| }; | |||
|
|
|||
| // Tags for denoting Sass list delimiters | |||
| enum Sass_List_Delimiter { | |||
There was a problem hiding this comment.
I'm not too excited about the wording "delimiter". I only use delimiter for stuff that goes between things and not around. Maybe that's just me? But I would have preferred something as Sass_List_Type. IMO it is confusing to have a delimiter and a separator.
| ADD_PROPERTY(bool, from_selector) | ||
| public: | ||
| List(ParserState pstate, | ||
| size_t size = 0, enum Sass_Separator sep = SASS_SPACE, bool argl = false) | ||
| size_t size = 0, enum Sass_Separator sep = SASS_SPACE, bool argl = false, | ||
| enum Sass_List_Delimiter delimiter = SASS_NO_DELIMITER) |
There was a problem hiding this comment.
Since this changes the function prototype anyway, I would prefer to have the "official" arguments before the "libsass" specific ones (size, sep, del, argl).
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#2134 rebased on master.
Closes #2134
Fixes #2073
Spec sass/sass-spec#1045