Skip to content

Keep the port in rdoc-image URLs - #1847

Open
Shaisolaris wants to merge 1 commit into
ruby:masterfrom
Shaisolaris:image-url-port
Open

Shaisolaris wants to merge 1 commit into
ruby:masterfrom
Shaisolaris:image-url-port

Conversation

@Shaisolaris

Copy link
Copy Markdown

An image link whose URL has a port was split at the colon before the port. For example, the Markdown

![logo](http://localhost:8080/logo.png)

rendered as

<img src="http://localhost" alt="8080/logo.png:logo">

The same happens for rdoc-image:http://localhost:8080/logo.png and {rdoc-image:http://example.com:8080/a.jpg:alt}[...] in RDoc markup.

handle_RDOCLINK split the text after rdoc-image: at the first colon not followed by /. This change matches the optional scheme://host:port part first and then splits the URL and the alt text at the next colon. Alt text that contains colons, such as ![a:b](c.png), still works as before.

Tests:

  • Added test_convert_TIDYLINK_image_external_with_port and test_convert_RDOCLINK_image_external_with_port_without_alt. Both fail on master and pass with this change.
  • test/rdoc/markup/to_html_test.rb, to_markdown_test.rb, rdoc_markdown_test.rb, heading_test.rb and to_html_snippet_test.rb pass locally (Ruby 3.3). I did not run the full suite or RuboCop.

A colon inside the query string of an image URL (for example ?t=1:2) is still ambiguous with the alt separator, and this change does not address it.

An image link whose URL has a port, such as
![logo](http://localhost:8080/logo.png) in Markdown or
rdoc-image:http://localhost:8080/logo.png, was split at the colon before
the port, so the image src became http://localhost and the rest of the
URL went into the alt text.

Split the URL and the alt text after the optional scheme, host and port
instead of at the first colon not followed by a slash.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant