-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tool: avoid including leading spaces in the Location hyperlink #11735
Conversation
When I do |
alacritty, foot and kitty include the space. I believe Konsole disables escape sequences for links by default. |
Ah, now I see the problem. Isn't this patch simpler? diff --git a/src/tool_cb_hdr.c b/src/tool_cb_hdr.c
index aab96ae03..b1bad12e0 100644
--- a/src/tool_cb_hdr.c
+++ b/src/tool_cb_hdr.c
@@ -399,12 +399,12 @@ void write_linked_location(CURL *curl, const char *location, size_t loclen,
if(!strcmp("http", scheme) ||
!strcmp("https", scheme) ||
!strcmp("ftp", scheme) ||
!strcmp("ftps", scheme)) {
- fprintf(stream, LINK "%s" LINKST "%.*s" LINKOFF,
- finalurl, (int)loclen, location);
+ fprintf(stream, " " LINK "%s" LINKST "%.*s" LINKOFF,
+ finalurl, (int)loclen, loc);
goto locdone;
}
/* Not a "safe" URL: don't linkify it */
No it doesn't. At least I get it linkified just fine. |
Or maybe this is even better: diff --git a/src/tool_cb_hdr.c b/src/tool_cb_hdr.c
index aab96ae03..878e8a563 100644
--- a/src/tool_cb_hdr.c
+++ b/src/tool_cb_hdr.c
@@ -399,12 +399,12 @@ void write_linked_location(CURL *curl, const char *location, size_t loclen,
if(!strcmp("http", scheme) ||
!strcmp("https", scheme) ||
!strcmp("ftp", scheme) ||
!strcmp("ftps", scheme)) {
- fprintf(stream, LINK "%s" LINKST "%.*s" LINKOFF,
- finalurl, (int)loclen, location);
+ fprintf(stream, " " LINK "%s" LINKST "%s" LINKOFF "\n",
+ finalurl, copyloc);
goto locdone;
}
/* Not a "safe" URL: don't linkify it */
|
/cc @dfandrich |
I had thought of fixing it like that. Is there a chance that not specifying the length will lead to bugs? |
Not that I can spot since |
The original intention was to preserve the whitespace so the user gets the same output with or without styling enabled (modula the styling, of course). The proposed patch strips leading whitespace and replaces it with a single character, which is often, but not always, the same. I didn't bother eliminating the leading whitespace from the linked section, but I can see how it could be annoying on some terminals. What I would probably do instead is count the amount of whitespace stripped in the
This is untested but the idea is to print the leading whitespace as-is, then the rest of the linkification without that leading space. |
5018ef7
to
2ec5d1b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works for me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
skip tabs
Co-authored-by: Dan Fandrich <dan@coneharvesters.com>
2ec5d1b
to
ad7d832
Compare
Thanks! |
Co-authored-by: Dan Fandrich <dan@coneharvesters.com> Closes curl#11735
This PR ensures that leading whitespaces are not marked as part of the Location URL.
Before:
After: