Selector functions #1064

onedayitwillmake · 2015-04-07T18:14:12Z

Please note: The code in this pull request was written by myself, and is does not represent my employer.

updated 4/08/15
updated 4/09/15

I rebased the PR #1019 and removed Xcode references.

I have 8 of the 8 built, I started from the bottom for some reason.
Bold means they are in and passing the test.

I'm still actively working on this - however being my second PR and new to the library I wanted to make sure anyone could catch any silly things I was doing.

selector-nest($selectors...)
selector-append($selectors...)
selector-extend($selector, $extendee, $extender)
selector-replace($selector, $original, $replacement)
selector-parse($selector)
simple-selectors($selector)
is-superselector($super, $sub)
selector-unify($selector1, $selector2)

Some notes:

Added stub functions
Passing 'Contextualize' to `Native_Function signature in BUILT_IN(name) Expression*
Added ARGSELL to compliment ARG, ARGR, ARGM since all the selector functions rely on that so heavily
Had to do some hairy things to get Selector_List::selector_unify and Selector_List::selector_unify to work
Added public static methods version of Extend's subweave, for now I named it StaticSubweave just to make sure I don't step on any toes, will remove floating one if that seems fine to everyone
Added a naiveTrim function to Node that removes any duplicate selectors (this is for Complex_Selector::unify_with was returning correct results but the final shared selector would sometimes appear twice, and blindly adding everything else in the collection (I could not get node's existing trim implementation to work - I Also could not find any references to it be used.
Added a public static version of Extend's extendSelectorList
Added param isReplace to Extend::extendSelectorList - which does not add the Selector if an extension takes place.
Added Vectorized<T>::first to compliment Vectorized<T>::back in order to make selector_append code more clear,

mgreter · 2015-04-07T18:27:49Z

Hey @onedayitwillmake great work so far 👍

I guess it is already passing quite a few todo tests? I also expect a few other extend bugs to be solved by this work, since it should fix/improve the is_superselector function. Maybe you have such a list? Otherwise I could add it since I have pretty much everything in place to generate it automatically with perl-libsass (IMO it should also be possible via sassc, but I never use it)!

Will definitely give it a thorough review after we released 3.2 🎆

coveralls · 2015-04-07T18:37:39Z

Coverage decreased (-1.58%) to 79.06% when pulling a7e93f8 on onedayitwillmake:selector_functions into 0135278 on sass:master.

mgreter · 2015-04-07T19:09:07Z

Gave it a whirl and these tests pass now on my side 😃

selector-functions/extend
selector-functions/parse
selector-functions/replace
selector-functions/extend/nested
selector-functions/is_superselector/any_is_not_superselector_of_different_prefix
selector-functions/is_superselector/complex_superselector
selector-functions/is_superselector/compound_superselector
selector-functions/is_superselector/current_is_superselector_with_identical_innards
selector-functions/is_superselector/current_is_superselector_with_subselector_innards
selector-functions/is_superselector/descendant_is_superselector_of_child
selector-functions/is_superselector/following_sibling_is_superselector_of_next_sibling
selector-functions/is_superselector/leading_combinator
selector-functions/is_superselector/matches_is_not_superselector_of_any
selector-functions/is_superselector/matching_combinator
selector-functions/is_superselector/not_is_not_superselector_of_non_unique_selectors
selector-functions/is_superselector/reflexivity
selector-functions/is_superselector/selector_list_subset
selector-functions/is_superselector/trailing_combinator

onedayitwillmake · 2015-04-07T19:23:04Z

@mgreter Would be great if you could generate it!

@mgreter @xzyfer Re: $selector-append
I have the function working with functionally correct output, however in ruby-sass the output is ordered alphabetically.

To give an example:

input: selector-append(".a", ".b, .c", ".de, .df, foo");
// ruby-sass
.a.b.de, .a.b.df, .a.bfoo, .a.c.de, .a.c.df, .a.cfoo;
// libsass 
.a.b.de, .a.c.de, .a.b.df, .a.c.df, .a.bfoo, .a.cfoo;

~~I could sort all the Complex_Selectors in the Selector_List, after the operation - but I'm not sure if that would be the right thing to do. Any advice?~~

Whoops, swapping the iteration order of the loop fixes the issue.
See: 7ccaece

onedayitwillmake · 2015-04-08T16:48:51Z

@mgreter @xzyfer Alright! Implemented selector-nest to make the full set

coveralls · 2015-04-09T20:39:04Z

Coverage decreased (-4.4%) to 76.24% when pulling 189fa0d on onedayitwillmake:selector_functions into 0135278 on sass:master.

coveralls · 2015-04-09T21:58:51Z

Changes Unknown when pulling 0c606ea on onedayitwillmake:selector_functions into * on sass:master*.

onedayitwillmake · 2015-04-09T22:00:09Z

@mgreter @xzyfer Do you guys have any advice on fixing the build errors in some of the travis-ci builds? It looks like it's failing on the issue_1075, in only some of the builds

xzyfer · 2015-04-10T03:43:53Z

@onedayitwillmake if you apply this change you should be good

coveralls · 2015-04-10T16:56:02Z

Changes Unknown when pulling 1e71a09 on onedayitwillmake:selector_functions into * on sass:master*.

onedayitwillmake · 2015-04-13T17:11:48Z

@xzyfer @mgreter Just curious if there was any movement on this, I implemented the .travis.yml modifications, should I just rebase?

xzyfer · 2015-04-14T00:29:17Z

@onedayitwillmake yes please rebase on master and resolve any conflicts.

It also looks like this branch has had a lot of merges with master which means it cannot be merged in it's current state. A rebase may help push out the unwanted commits, otherwise we'll need to find another way to clean up this branch.

We rely heavily on git forensics to track down bug and regressions. This requires a clean commit history.

onedayitwillmake · 2015-04-14T00:30:32Z

@xzyfer Gotcha, makes sense.
Besides the one on the bottom, which other ones do you see have merges with master?

xzyfer · 2015-04-14T00:39:50Z

I can see commits from myself and @mgreter which don't belong here. These were presumably introduced in a merge.

xzyfer · 2015-04-19T16:55:40Z

@onedayitwillmake have you had a chance to clean up this PR? You can probably squash this down to a few commits.

onedayitwillmake · 2015-04-20T19:37:04Z

@xzyfer I will give it another go tomorrow morning, you reckon I should just squash it to a few key commits and rebase from there?

xzyfer · 2015-04-21T00:01:06Z

@onedayitwillmake yes I do. However I expect you will into issues due to the merge commits.

# The first commit's message is: Moved naiveTrim to Node # The 2nd commit message will be skipped: # Fixed some non-wrapped DEBUG statements causing compile errors

onedayitwillmake · 2015-04-24T00:34:11Z

@xzyfer Looks like something else broke on this one, saying I have a duplicate function however I'm not seeing that on my end. Maybe I'm just bad at pull-request, what do you suggest

xzyfer · 2015-04-24T14:53:09Z

ast.cpp

+
+
+  // For every selector in RHS, see if we have /any/ selectors which are a super-selector of it
+  bool Selector_List::is_superselector_of(Sass::Selector_List *rhs) {


I believe this is cause of the CI error. See line 501 above.

xzyfer · 2015-04-24T14:53:56Z

Impressive git work to fix this branch.

Please remove the debug code.

coveralls · 2015-04-29T23:22:13Z

Coverage decreased (-1.8%) to 78.46% when pulling 74600d3 on onedayitwillmake:selector_functions into 20d8870 on sass:master.

onedayitwillmake · 2015-04-29T23:27:23Z

@xzyfer Thanks for heads up, I removed them

onedayitwillmake · 2015-05-02T19:20:30Z

@mgreter Any movement on this one?

xzyfer · 2015-05-03T05:00:03Z

ast.cpp

+
+      for (size_t i = 0, L = extender->length(); i < L; ++i) {
+        // let's test this out
+        cerr << "REGISTERING EXTENSION REQUEST: " << (*extender)[i]->perform(&to_string) << " <- " << compound_sel->perform(&to_string) << endl;


debug code?

xzyfer · 2015-05-03T05:06:39Z

@onedayitwillmake I've left some nitpick feedback for now. This will be a closer look when we're ready to start working on 3.3. Thanks heaps for all your work here, it's really appreciated.

One suggestion I have is to try to make the diff as clean as possible so we can focus on the changes that are relevant i.e. avoid unnecessary white changes, remove debug or commented out code

Based on sass#1064

xzyfer · 2015-07-08T13:29:52Z

Superseded #1261

mgreter added Dev - WIP Discussion - Proposal labels Apr 7, 2015

mgreter added this to the 3.2.1 milestone Apr 7, 2015

xzyfer mentioned this pull request Apr 8, 2015

[WIP] Selector functions #1019

Closed

onedayitwillmake force-pushed the selector_functions branch from 1e71a09 to fedea6b Compare April 23, 2015 23:37

onedayitwillmake added 8 commits April 23, 2015 16:38

Added stub selector functions

891a29f

Initial version of selector-parse

48c39a6

Initial version of simple-selectors

27010fd

Added selector function is-super-selector

7a374fd

Initial half working version of selector-unify

d24be46

Working selector_unify but results not compacted

4a910b2

# This is a combination of 2 commits.

badb7a2

# The first commit's message is: Moved naiveTrim to Node # The 2nd commit message will be skipped: # Fixed some non-wrapped DEBUG statements causing compile errors

Moved naiveTrim to Node

90f7e0b

xzyfer reviewed Apr 24, 2015
View reviewed changes

Removed DEBUG code comments

74600d3

onedayitwillmake force-pushed the selector_functions branch from fedea6b to 74600d3 Compare April 29, 2015 23:16

xzyfer modified the milestones: 3.2.2, 3.3 May 1, 2015

xzyfer reviewed May 3, 2015
View reviewed changes

mgreter pushed a commit to mgreter/libsass that referenced this pull request May 12, 2015

Implement is_superselector sass function [RFC] [WIP]

1de0132

Based on sass#1064

mgreter mentioned this pull request May 12, 2015

Implement is_superselector sass function #1202

Merged

onedayitwillmake mentioned this pull request Jun 4, 2015

Selector Functions #1261

Closed

7 tasks

xzyfer closed this Jul 8, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Selector functions #1064

Selector functions #1064

onedayitwillmake commented Apr 7, 2015

mgreter commented Apr 7, 2015

coveralls commented Apr 7, 2015

mgreter commented Apr 7, 2015

onedayitwillmake commented Apr 7, 2015

onedayitwillmake commented Apr 8, 2015

coveralls commented Apr 9, 2015

coveralls commented Apr 9, 2015

onedayitwillmake commented Apr 9, 2015

xzyfer commented Apr 10, 2015

coveralls commented Apr 10, 2015

onedayitwillmake commented Apr 13, 2015

xzyfer commented Apr 14, 2015

onedayitwillmake commented Apr 14, 2015

xzyfer commented Apr 14, 2015

xzyfer commented Apr 19, 2015

onedayitwillmake commented Apr 20, 2015

xzyfer commented Apr 21, 2015

onedayitwillmake commented Apr 24, 2015

xzyfer Apr 24, 2015

xzyfer commented Apr 24, 2015

coveralls commented Apr 29, 2015

onedayitwillmake commented Apr 29, 2015

onedayitwillmake commented May 2, 2015

xzyfer May 3, 2015

xzyfer commented May 3, 2015

xzyfer commented Jul 8, 2015



		// For every selector in RHS, see if we have /any/ selectors which are a super-selector of it
		bool Selector_List::is_superselector_of(Sass::Selector_List *rhs) {

Selector functions #1064

Selector functions #1064

Conversation

onedayitwillmake commented Apr 7, 2015

mgreter commented Apr 7, 2015

coveralls commented Apr 7, 2015

mgreter commented Apr 7, 2015

onedayitwillmake commented Apr 7, 2015

onedayitwillmake commented Apr 8, 2015

coveralls commented Apr 9, 2015

coveralls commented Apr 9, 2015

onedayitwillmake commented Apr 9, 2015

xzyfer commented Apr 10, 2015

coveralls commented Apr 10, 2015

onedayitwillmake commented Apr 13, 2015

xzyfer commented Apr 14, 2015

onedayitwillmake commented Apr 14, 2015

xzyfer commented Apr 14, 2015

xzyfer commented Apr 19, 2015

onedayitwillmake commented Apr 20, 2015

xzyfer commented Apr 21, 2015

onedayitwillmake commented Apr 24, 2015

xzyfer Apr 24, 2015

Choose a reason for hiding this comment

xzyfer commented Apr 24, 2015

coveralls commented Apr 29, 2015

onedayitwillmake commented Apr 29, 2015

onedayitwillmake commented May 2, 2015

xzyfer May 3, 2015

Choose a reason for hiding this comment

xzyfer commented May 3, 2015

xzyfer commented Jul 8, 2015