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

Implemented named arguments #268

Merged
merged 2 commits into from
Apr 30, 2012
Merged

Implemented named arguments #268

merged 2 commits into from
Apr 30, 2012

Conversation

jamesfoster
Copy link
Contributor

Hey Alexis,

Here's an implementation of named parameters. For example:

.mixin(@a: 1, @b: 2, @c: 3, @d: 4, @e: 5) {
  ...
}

.class {
  .mixin(1, 2, 3, 4, 10);
  .mixin(@e: 10);
}

If you wanted to override the 5th argument you would need to know the default value for the first 4 before supplying the 5th and if the defaults changed you now have to find everywhere where you've done this.

Not good.

Now you just specify the name of the argument you want to override.

It might be nice to throw an error if you place any named arguments before any "positional" arguments. as it stands it works regardless but you may get unexpected results.

Cheers

James

@paulyoung
Copy link

This is great!

@arharp
Copy link

arharp commented May 17, 2011

Yeah, this looks awesome.

@dimensia
Copy link

Nice!

@jamesfoster
Copy link
Contributor Author

This should really be a second pull request. Just "fully" implemented the & parent selector.

Enjoy

@paulyoung
Copy link

Another killer feature!

Here's the usage. This:

#box {
    #other-box {
        margin: 10px 0 0;
    }
}

.ie7 {
    #box {
        #other-box {
            margin: 5px 0 0;
        }
    }
}

Becomes:

#box {
    #other-box {
        margin: 10px 0 0;

        .ie7 & {
            margin: 5px 0 0;
        }
    }
}

This removes the need for duplicate rules in separate blocks, and also overcomes the need to match the structure exactly within those blocks to overcome specificity issues. e.g. This wouldn't have worked for the above:

.ie7 {
    #other-box {
        margin: 5px 0 0;
    }
}

Nice work!

@jamesfoster
Copy link
Contributor Author

Just rebased to make it easier to merge. Also squashed the named arguments feature into a single commit.

Original branch at jamesfoster@req268

@revolunet
Copy link
Contributor

OMG +1 great work

@cowboyd
Copy link
Contributor

cowboyd commented Jun 7, 2011

This has been on my wish list for a very long time, and I'd love to see it in one form or another.

@thatdutchguy
Copy link

Do want

@leeoniya
Copy link

leeoniya commented Jun 7, 2011

+1

2 similar comments
@Evgenus
Copy link

Evgenus commented Jun 11, 2011

+1

@chelmertz
Copy link

+1

@oknoway
Copy link

oknoway commented Jun 18, 2011

Hard to think of anything I've wanted more than a parent selector. Great work!

@joeldbirch
Copy link

Really looking forward to being able to use this.

@jamesfoster
Copy link
Contributor Author

Thanks for all the comments. This is one of those things I've wanted for a while myself too. my friend brought it up some time later and the solution just hit me.

Not sure what's holding up the merge but @cloudhead assures me it's coming.

@csnover
Copy link
Contributor

csnover commented Jun 27, 2011

773229f applies cleanly against the current master but does not work (causes a JS error); 2e042e9 applies and works fine. I wonder if it would be easier to get this landed if it’s split into two pull reqs?

@jamesfoster
Copy link
Contributor Author

ok. thanks for this. I'll split the requests up and add the above case to the tests and fix it.

@cowboyd
Copy link
Contributor

cowboyd commented Sep 1, 2011

@jamesfoster Your parent selector fixes were merged, but did you ever resubmit the named parameters enhancement. I really, really, really think that this is a killer feature, and deserves another PR.

@Spaxe
Copy link

Spaxe commented Feb 2, 2012

Any progress on this ticket?

@paulyoung
Copy link

Would love to see named arguments added.

@donaldharvey
Copy link

Currently working on an updated version of this here; will be submitting another pull request after I've tested it a bit more thoroughly.

@regisphilibert
Copy link

Woah, I'm tweeting this, cheers mate!

@geddski
Copy link

geddski commented Apr 25, 2012

What ever happened to this? I found myself needing named arguments and wishing LESS had it. Seeing a pull request open for a year that has it is pretty frustrating. I see why people are switching to stylus/sass.

…tor.

'''
.foo {
  .bar & {
    &:hover {
      color: orange;
    }
  }
}
'''

now outputs

'''
.bar .foo:hover {
  color: orange;
}
'''
@jamesfoster
Copy link
Contributor Author

Rebased and fixed issue with trailing space.

@ursbraem
Copy link

+1

cloudhead pushed a commit that referenced this pull request Apr 30, 2012
Implemented named arguments
@cloudhead cloudhead merged commit a2df119 into less:master Apr 30, 2012
@yanneves
Copy link

+1

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 this pull request may close these issues.

None yet