track: make --filename work with spaces#3785
Conversation
|
The fix works for me. Thanks for the quick turn around on getting this issue fixed. |
PastelMobileSuit
left a comment
There was a problem hiding this comment.
Had one minor piece of feedback, but otherwise this looks good! ✨
commands/command_track.go
Outdated
| @@ -91,18 +91,17 @@ ArgsLoop: | |||
|
|
|||
| // Generate the new / changed attrib line for merging | |||
| encodedArg := escapeAttrPattern(pattern) | |||
There was a problem hiding this comment.
It looks like we've merged this functionality into escapeGlobCharacters and so there's no longer any need for this call. I think we should remove it and, since escapeAttrPattern isn't being used anywhere else, we should remove that as well
There was a problem hiding this comment.
I think the way the diff has displayed is actually a bit misleading here. We want to use escapeAttrPattern in the case when we're not using --filename, since that intentionally doesn't escape glob characters, so *.png stays *.png in the .gitattributes file. But when we use --filename, we do want to escape glob characters, so we use escapeGlobCharacters. The latter just uses parts of the former because the order matters.
I'm wondering, though, if perhaps this would be easier to read as an if/else, where it would be more obvious that escapeAttrPattern is only used if trackFilenameFlag is off. Let me rewrite it that way and see if that's easier to understand.
There was a problem hiding this comment.
Ah of course! That makes sense. Thanks for switching this over to an if-else, it definitely makes things easier to read/understand
747f789 to
d418b23
Compare
When using "git lfs track --filename" with a filename with spaces, the brackets surrounding the character class for representing spaces were also escaped, leading to the space not being interpreted correctly. Update the escaping function we use to handle all of our escaping needs, performing the escaping in the correct order so that we can handle spaces correctly.
d418b23 to
fa0f81c
Compare
When using "git lfs track --filename" with a filename with spaces, the brackets surrounding the character class for representing spaces were also escaped, leading to the space not being interpreted correctly. Update the escaping function we use to handle all of our escaping needs, performing the escaping in the correct order so that we can handle spaces correctly.
/cc @gdlxn-ibm as reporter