opts/envfile: trim all leading whitespace in a line #15762
Conversation
GordonTheTurtle
added
status/0-triage
dco/no
GordonTheTurtle
removed
the
dco/no
|
testcases? |
|
Yeah, adding now. :) |
|
@duglin test added, which fails before this patch, but passes after patch. |
|
LGTM |
| // because it's common for editors to trim trailing whitespace | ||
| // from lines, which becomes annoying since that's the | ||
| // exact thing we need to test. | ||
| content += "\n " |
There was a problem hiding this comment.
Do we need a similar test, using a tab as whitespace?
Also, this looks to be adding trailing whitespace (the whitespace is appended to the content), not leading whitespace, is that correct?
There was a problem hiding this comment.
think I misread the description of the PR, got confused because of the "trim the first line" in your PR description
There was a problem hiding this comment.
Ah, I see the misunderstanding, but it says "trim the line first" NOT "trim the first line". ;)
tl;dr, if there's a line with only whitespace, docker would error before.
Do we need a similar test, using a tab as whitespace?
I could just add a \t into this line as well to cover both.
There was a problem hiding this comment.
Doh! You're right, probably combined with the "trim leading" title of the PR, I got completely lost LOL
I could just add a \t into this line as well to cover both
Saw you added it, thanks!
mattrobenolt
force-pushed
the
trim
branch
2 times, most recently
from
cb636d9
to
299f2be
Compare
mattrobenolt
changed the title
Signed-off-by: Matt Robenolt <matt@ydekproductions.com>
|
LGTM |
opts/envfile: trim all leading whitespace in a line
Rather than just trimming before a variable, we should trim the line first.
This prevents a line with whitespace only to not break docker.