Skip to content
This repository has been archived by the owner on Apr 23, 2021. It is now read-only.

Should padLeft pad "outwardly", or "left"? #6

Closed
ljharb opened this issue Sep 23, 2015 · 17 comments
Closed

Should padLeft pad "outwardly", or "left"? #6

ljharb opened this issue Sep 23, 2015 · 17 comments

Comments

@ljharb
Copy link
Member

ljharb commented Sep 23, 2015

Investigate use cases, and what other langs do. From @michaelficarra

@michaelficarra
Copy link
Member

I call this "alignment".

@ljharb
Copy link
Member Author

ljharb commented Sep 24, 2015

Things with the ability to specify both a length and a fillStr

Ruby has ljust and rjust, which does not conform to the current alignment:

'abc'.rjust(5, 'def') => 'deabc'
'abc'.rjust(6, 'def') => 'defabc'
'abc'.ljust(5, 'def') => 'abcde'
'abc'.ljust(6, 'def') => 'abcdef'

PHP has strpad, which conforms to the current alignment, "outwardly":

echo str_pad('abc', 5, 'de', STR_PAD_LEFT); => "deabc"
echo str_pad('abc', 6, 'de', STR_PAD_LEFT); => "dedabc"
echo str_pad('abc', 6, 'de', STR_PAD_RIGHT); => "abcde"
echo str_pad('abc', 6, 'de', STR_PAD_RIGHT); => "abcded"

Things that don't truncate

sed doesn't truncate, and the user specifies the alignment by their placement of &:

echo 'abc' | sed -e :a -e 's/^.\{1,5\}$/&de/;ta' => abcdede
echo 'abc' | sed -e :a -e 's/^.\{1,5\}$/de&/;ta' => dedeabc
echo 'abc' | sed -e :a -e 's/^.\{1,5\}$/ed&/;ta' => ededabc

Scala doesn't let me truncate or pad on the opposite side, and the number argument seems to be "number of occurrences of the fillStr":

"abc".padTo(6, "de") => "abcdedede"

Languages that only allow a single space, and thus don't apply

Java has String.format, which only pads with a single space:

String.format("%6s", "abc") => "   abc"
String.format("%-6s", "abc") => "abc   "

Smalltalk and Perl have sprintf which only fills with a single space.

Misc

Python also has ljust and rjust, but throws if you provide a multiple character fillStr:

'abc'.ljust(6, 'de') => TypeError: The fill character must be exactly one character long
'abc'.rjust(6, 'de') => TypeError: The fill character must be exactly one character long

Conclusion

I've found only two languages that both accept a multiple character fill string, and decide the alignment for you. One, PHP, follows the current proposal (outward alignment). The other, Ruby, deviates from the current proposal (fillString-order alignment, ie, left-to-right). I don't feel strongly about one over the other, so unless more languages are suggested/discovered that help move the needle, I'd prefer to keep the proposal as-is. Thoughts?

@michaelficarra
Copy link
Member

Ruby still appears to differ from this proposal. Ruby:

irb(main):002:0> 'foo'.rjust(5, 'bar')
=> "bafoo"

This proposal:

assert(padLeft('foo', 5, 'bar') === 'arfoo');

@ljharb
Copy link
Member Author

ljharb commented Sep 25, 2015

Ah yes, you're correct - my Ruby example didn't correctly indicate what was going on. I'll update my comment.

@ljharb
Copy link
Member Author

ljharb commented Sep 25, 2015

@michaelficarra Given this sole example of alignment with the fill string, how do you want to proceed? Are you comfortable following PHP's example? Or would you prefer to follow Ruby's example?

Alternatively, do you have any other languages to recommend investigating that would help decide this? As it is, I'm aware of one applicable language on either side, which means we can choose whichever we want.

@michaelficarra
Copy link
Member

Can you update the PHP example in the same way? I think the new test is much better.

@ljharb
Copy link
Member Author

ljharb commented Sep 25, 2015

Done!

@michaelficarra
Copy link
Member

Okay, so they both are aligning padding left, which is what I would have intuited and not the way that is described by these proposals. Are you going to update them to sync with Ruby/PHP?

@ljharb
Copy link
Member Author

ljharb commented Sep 25, 2015

Given the evidence, that seems reasonable. I'll update that shortly.

To illustrate the difference: "abc".padLeft(8, "def") => "efdefabc" versus "defdeabc". They both prepend one fillStr at a time, but once it's too large, the former truncates on the left, the latter truncates on the right. The former means the rightmost character of the fillStr is always present, the latter means the leftmost character of the fillStr is always present. The proposal will change to mean the latter for padLeft.

@ljharb
Copy link
Member Author

ljharb commented Sep 25, 2015

@bterlson came up with a clear example of why this matters: With this change, "abc".leftPad(10, "1234567890") will produce "1234567abc", which is clearly better than without this change, which would produce "4567890abc". In generic terms, string.padLeft(mask.length, mask) should do what one expects.

@zloirock
Copy link
Contributor

1234567abc which is clearly better than without this change

Strange logic. 4567890abc looks more clearly for me.

@bterlson
Copy link
Member

str.padLeft(mask.length, mask) seems to be a valid use-case. Can you share a motivating example for the current behavior?

@ljharb
Copy link
Member Author

ljharb commented Sep 25, 2015

I had no motivating example, I just thought that padLeft should do the inverse of what padRight was doing, so that the fillStrings would be mirrors of each other.

Regardless, the consistency with both Ruby and PHP (and the lack of any other example language that differs) completely trumps it in my mind, even without the very compelling "mask" use case.

@bterlson
Copy link
Member

I was referring to @zloirock. I'm curious how the current behavior makes more sense to him (presumably because it's useful for some purpose I haven't thought of!).

ljharb added a commit to es-shims/String.prototype.padStart that referenced this issue Sep 25, 2015
@zloirock
Copy link
Contributor

@bterlson yes, with mask example 1234567abc will be convenient, but it will break mirror consistency with padRight.

@ljharb
Copy link
Member Author

ljharb commented Sep 25, 2015

Even though 'abc' isn't symmetrical already?

@zloirock
Copy link
Contributor

For mask case enough mask.slice(0, -str.length) + str.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants