Added Padding extraction functions: top(@pad) right(@pad) bottom(@pad) left(@pad)

[scottrippey]

I've implemented these functions.
Please see #1100 for use cases and more info.

[scottrippey]

@agatronic @MatthewDL @gustavohenke Open for discussion:
Should this be implemented as 4 separate functions, or simply 1 function?

The current implementation has the top right bottom left functions delegated through a private helper, _extractPadding, so a refactor would take mere seconds.

Naming suggestions for the single function:

@shorthand: 1px 2px 3px;
pick( top | right | bottom | left , @shorthand)
extract( top | right | bottom | left, @shorthand)

I like extract, and I think it could possibly be used in the future for more difficult shorthand extractions. But for now, the implementation will only support extracting from margin & padding shorthand.

[scottrippey]

In addition to extracting from margin & padding shorthand top right bottom left, this could also be used to extract from the border-radius shorthand: top-left top-right bottom-right bottom-left, using the same exact logic as margin & padding.
Example:

@shorthand: 1px 2px 3px;
extract( top-left | top-right | bottom-right | bottom-left , @shorthand)

I actually think this use-case is as valid as the margin & padding use-case, so I will soon perform the refactoring, unless I hear any objections?

[ricardobeat]

Your patch is using tabs instead of spaces, you probably want to check that.

What about a simpler, index-based approach?

ricardobeat/less.js@b9c68c2

[gustavohenke]

@ricardobeat , the "index based approach" was already discussed in #1100...

@scottrippey,
I do prefer only one, but I can't speak by @agatronic, who's officialy maintaining less.js :D

And, no mather the approach you use, it would be useful for many dimension extracting:

• margin
• padding
• border-radius
• border-width
• background-position
• top, left, bottom, right

[scottrippey]

@ricardobeat Re: tabs vs spaces: The first commit was with tabs, but I "normalized whitespace" and fixed the mistake ... did you spot any other mistakes I might have missed?

@gustavohenke Earlier, @agatronic said "I considered 1 function better than 4 (more succinct). " So I'll refactor accordingly.

If it's just 1 function, then it'll be a piece of cake to implement support for more mapping parameters:

extract(  [0-9]+ | top | right | bottom | left | top-left | top-right | bottom-right | bottom-left , @shorthand)

I need help with 2 important design decisions, though:

• Most appropriate method name? The only suggestions so far are extract and pick, My opinion: extract seems most appropriate.
• Should the index-based approach be 1-based or 0-based? My opinion: 1-based seems most appropriate.

[lukeapage]

Please wait to see if we get some more feedback from @MatthewDL

and if he really doesn't like it we'll discuss with @cloudhead

[scottrippey]

@agatronic I'll wait to hear from those gentlemen :)

[matthew-dean]

I agree that if we cannot know what kind of value we are extracting, then allowing naming like "top", "left", etc. is inappropriate and adds bloat.

If this is essentially to pull a sub-value from a space-separated shorthand variable, it's probably a tiny addition. We already do magic with making an argument collection space separated, so this seems like a nice inverse. (That is, you could / should be able to do extract(@arguments, @number) to set a variable to pick which argument passed into a mixin is assigned.

So, kill off the keywords, and it seems fine. I like "extract" better than "pick".

CSS uses 1-based indexes (nth-child), so I'm inclined that way as well.

@agatronic Agree with that?

[matthew-dean]

To further clarify, I'm 100% against adding a slew of extraction functions. That's a pile of unnecessary confusion for what should be a very small feature, if it exists at all. extract() or nothing.

[ricardobeat]

@gustavohenke discussion was moved here, and the consensus on #1100 was that supporting keywords is too much bloat. That's why I suggested the simpler index-based implementation. Adding a 30-line method seems like overkill.

Btw, wouldn't get(@margins, 1) be a suitable name?

[scottrippey]

@MatthewDL You certainly have good points. The index-based part is just 2 lines of code. The keyword-based approach (including the extra shorthand logic) was starting to get out-of-hand. LESS currently doesn't "understand" CSS this way, it just "builds" CSS, so maybe that's the reason why this feels out-of-place.

I liked the semantics of extract(left, @pad), but would be fine with extract(4, @pad).
I'm also OK with the limitation of not supporting any shorthand logic ... in other words, if I need to extract the left value from @pad, then I can't allow @pad to have <= 3 values -- it must have 4 values or it won't compile.

[lukeapage]

I liked being able to pass TRLB, but it did suffer terribly from the potential of not working for some sets of properties in the future. I thought that bad side was worth the niceness of it, but I'm willing to conceded, espescially since the original proposer doesn't mind.

extract is fine by me. get seems too generalised.

nth-child - when using 2n + 1 - n starts at 0 in order that the first result is odd, e.g. 1 which is the first child.. so that seems contradictory. I can see why the first-child is 1 - because 1 is odd, must human counting starts at 1 and it would seem strange if a css writer used a +1 exression to indicate an even row selection. 0th child vs 1st child doesn't make sense. I've been trying to find any other example of indexing in css and I can't. counter-reset resets to 0, but thats not much of an argument. :eq is 0 based, but thats jQuery.

I'd rather say what would a less user expect it to be.. 0 based or 1 based? Personally I'd expect it 0-based.

[scottrippey]

Most of my design decisions were based on the readability of the LESS. extract(top, @pad) obviously reads "extract top from @pad" ... I think the same holds true for an index, "extract 1st from @pad".

Since @pad is not an array, nor a list ... it is a group of values, and I would verbally reference those values as "1st, 2nd, 3rd, etc"

[lukeapage]

Ok, 1-based it is.

[matthew-dean]

Just one thing: I would have expected the syntax to be extract(@pad, 1) not extract(1, @pad). It seems more common to first pass the thing to be altered. I'd give similar examples from other LESS functions, but LessCSS.org appears to be down right now (for me anyway at this moment).

[matthew-dean]

From memory, the contrast function is like that. First, pass in the vars, then the numerical value of threshold.

[ricardobeat]

unit() and all color manipulation functions (saturate, fade, et al) use the (subject, args) argument order.

It's the standard order in most languages, since at the moment you want to add a 2nd argument you get a mess: extract(1, @pad, otherstuff?). It's also just an artifact of the language limitations, what we really want to do is @margins[1], @margins.extract(1) or something like that, so it would be natural for it to look like extract.call(@margins, 1).

[lukeapage]

lets swap them round

raised #1119 so I don't forget

[scottrippey]

Ok, good points, extract(@pad, 1) is the way to go.
I've committed the change, but I see that this issue was closed without pulling the code ... does that mean I need to create a new pull request?
Update: nevermind, I didn't know that 1.4.0 was merged to master, and now I see the extract function has been merged.

[lukeapage]

yes, I rebased the pull and pushed your commits together as it didn't make sense to have so many commits for what ended up being so tiny. Sorry if that messes with your history!

[scottrippey]

I'm new to collaborating via Git & GitHub, so I need to learn how to squash commits, rebase, pull upstream, etc...