Conversation
I encountered some problematic URLs, and was delighted to see that they were already fixed. I figured I may as well add them to the changelog. For the record, URLs with no path used to be rejected. That is arguably correct, but command line git accepts them. URLs with a path of / and a non-standard port used to have their port completely ignored!
There is no git_stash_apply_flags_t above.
pks-t
left a comment
There was a problem hiding this comment.
Thanks for your changes! The flags change is definitely good, but I'm not too sure about the other change. Didn't we handle the URLs you have provided as examples before already?
| * libgit2 now correctly handles more URLs, such as | ||
| `http://example.com:/repo.git` (colon but no port), | ||
| `http://example.com` (no path), | ||
| and `http://example.com:8080/` (path is /, nonstandard port). |
There was a problem hiding this comment.
Didn't we handle both URLs with no path and / path with nonstandard port just fine before? I'm not too sure about this, but I thought the only change we introduced was that we are now able to parse empty ports with that release.
There was a problem hiding this comment.
Parsing empty ports may have been the motivating case, but neither of those other cases worked for me. Please see the commit message, where I documented what the previous behavior was. Should I provide reproduction instructions as well?
For the record, I bisected the change in behavior to c6ab183. It turns out they still aren't actually fully working yet: #5321. But I hope that that will get fixed before the next release.
There was a problem hiding this comment.
Ah, perfect! I didn't expect nice commit messages, but here they are. So no, this is perfectly fine in that case and makes sense to me.
| unsigned int version; | ||
|
|
||
| /** See `git_stash_apply_flags_t`, above. */ | ||
| /** See `git_stash_apply_flags`, above. */ |
| * libgit2 now correctly handles more URLs, such as | ||
| `http://example.com:/repo.git` (colon but no port), | ||
| `http://example.com` (no path), | ||
| and `http://example.com:8080/` (path is /, nonstandard port). |
There was a problem hiding this comment.
Ah, perfect! I didn't expect nice commit messages, but here they are. So no, this is perfectly fine in that case and makes sense to me.
No description provided.