Change backslash to forward slash on windows #66
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #66 +/- ##
==========================================
+ Coverage 77.82% 78.18% +0.36%
==========================================
Files 4 4
Lines 239 243 +4
==========================================
+ Hits 186 190 +4
Misses 53 53 ☔ View full report in Codecov by Sentry. |
|
thanks for the contribution! It would be good to first see a failing test without this change, then to see the test succeed after the change |
danieleades
force-pushed
the
fix_backslash
branch
from
9725468
to
805df79
Compare
christianjann
force-pushed
the
fix_backslash
branch
from
805df79
to
bde4fb1
Compare
christianjann
force-pushed
the
fix_backslash
branch
from
bde4fb1
to
fc16253
Compare
Not sure yet how to best test this, how to check the generated html for the correct path, might be better to just merge and unblock other windows users and maybe think about some generic end-2-end tests later. |
danieleades
force-pushed
the
fix_backslash
branch
from
fc16253
to
0f8a2ae
Compare
|
unfortunately testing on windows is blocked on terrastruct/d2#2044 |
| @@ -151,7 +151,11 @@ impl Backend { | |||
| Event::Start(Tag::Paragraph), | |||
| Event::Start(Tag::Image { | |||
| link_type: LinkType::Inline, | |||
| dest_url: rel_path.to_string_lossy().to_string().into(), | |||
There was a problem hiding this comment.
is this definitely necessary?
markdown should be platform-independent. I don't believe backslashes are required in filepaths embedded in markdown, even in a windows environment
There was a problem hiding this comment.
is this definitely necessary? markdown should be platform-independent. I don't believe backslashes are required in filepaths embedded in markdown, even in a windows environment
No they should not be there, but somehow the backslash came in and in the html it was then escaped with some %HexNumber, will try to run the integration tests on Windows and paste the output
https://users.rust-lang.org/t/path-to-url-easiest-way/28632
Something like this might also work: path.components().map(|c| urlencode(c.as_bytes())).join("/")
There was a problem hiding this comment.
Running cargo test without and with my commit:
Unfortunately temp file is gone but the test proves the expected string is not there ;)
Change backslash to forward slash for windows compatibility rel_path might be something like d2\1.1.svg which does not work in html
danieleades
force-pushed
the
fix_backslash
branch
from
0f8a2ae
to
3e66980
Compare
Change backslash to forward slash for windows compatibility
rel_path might be something like d2\1.1.svg which does not work in html