Skip to content
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

Consider CRLF when splitting code lines #991

Merged
merged 3 commits into from
Mar 27, 2019
Merged

Consider CRLF when splitting code lines #991

merged 3 commits into from
Mar 27, 2019

Conversation

notZaki
Copy link
Contributor

@notZaki notZaki commented Mar 25, 2019

I noticed that code lines ending with # hide were not getting hidden in the output.
The problem went away if I switched from CRLF to LF line endings.
This might be related to issue #613

I think the issue is caused by split(code, '\n') (line 680 in expanders.jl) which assumes LF line ending.
For CRLF input, code containing ... # hide gets split to ... # hide\r and this causes the regex on line 681 to not match---this is because $ does not match \r [ref]

This PR fixes that by splitting code with r"\r?\n".


Minimum working example:

  • make.jl
using Documenter
makedocs(sitename="Example")
  • src/index.md (using CRLF)
# Example

```@example
import Random # hide
Random.seed!(1) # hide
A = rand(3, 3)
b = [1, 2, 3]
A \ b
Random.seed!(2); nothing # hide
  • Ouput using master (note that last line still gets hidden correctly):
    image

  • Output using PR
    image

  • If LF is used, then both master and PR have the same output where code is hidden correctly

@mortenpi mortenpi added this to the 0.22.0 milestone Mar 27, 2019
Copy link
Member

@mortenpi mortenpi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks! LGTM!

Could you also add a bugfix entry to CHANGELOG.md for this, under v0.22.0?

CHANGELOG.md Outdated Show resolved Hide resolved
@mortenpi
Copy link
Member

Some parallel processing here 😆 Thanks! Will merge when tests go green.

@notZaki
Copy link
Contributor Author

notZaki commented Mar 27, 2019

Updated changelog.
Didn't know if the policy was to squash commits or not, so I forced push the changes to avoid unnecessary commit messages.

@mortenpi
Copy link
Member

I squash merge PRs usually, so no need to squash them by hand.

@mortenpi mortenpi merged commit b62f6ab into JuliaDocs:master Mar 27, 2019
@notZaki notZaki deleted the CRLF-hide branch March 27, 2019 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants