Make sure we have LF line endings only. #791

fredrikekre · 2018-08-13T13:11:05Z

fix #749, fix #790

fredrikekre · 2018-08-13T13:11:47Z

@scls19fr can you try this branch? You can pkg> add Documenter#fe/fix-parsing from the Pkg REPL.

fredrikekre · 2018-08-13T13:12:25Z

@Datseris maybe you can try this branch too, if you still can reproduce this problem.

Datseris · 2018-08-13T13:13:33Z

Im on it.

scls19fr · 2018-08-13T13:16:01Z

LGTM. Thanks @fredrikekre for this fix

fredrikekre · 2018-08-13T13:16:15Z

src/Utilities/Utilities.jl

    # Drop `skip` leading lines from the code block. Needed for deprecated `{docs}` syntax.
    code = string(code, '\n')
    code = last(split(code, '\n', limit = skip + 1))
-    # Check whether we have windows-style line endings.
-    offset = occursin("\n\r", code) ? 2 : 1


I suspect the error was from this beeing "\n\r" and not "\r\n", but I figured we might as well just use "\n" internally.

The code here is not used when writing back things (like when fixing up doctests)?

I don't think so. That code kinda assumes \n anyways.

And also, this local code argument does not leak out of this parser function anyways.

fredrikekre · 2018-08-13T13:16:32Z

Perfect, thanks for confirming.

Datseris · 2018-08-13T13:24:09Z

Amazing @fredrikekre ! It works without any problems!

fredrikekre · 2018-08-13T14:08:20Z

src/Utilities/Utilities.jl

@@ -114,11 +114,11 @@ parsed from.
 The keyword argument `skip = N` drops the leading `N` lines from the input string.
 """
 function parseblock(code::AbstractString, doc, page; skip = 0, keywords = true)
+    # work only with LF line endings internally
+    code = replace(code, "\r\n" => "\n")


Actually, this could be dangerous if someone have "\r\n" in a string. Should revert this and let the Julia parser decide this kind of things instead.

fredrikekre · 2018-08-13T14:14:59Z

@Datseris or @scls19fr sorry, but can you check the updated version too? pkg> up from the REPL if you still have this branch pkg> added, otherwise just add again as above.

scls19fr · 2018-08-13T15:47:53Z

It's broken on my side

Datseris · 2018-08-13T20:35:01Z

Hm That's weird @scls19fr . @fredrikekre I can confirm that this is still LGTM on Windows 10. I am also testing it with a documentation that was problematic, i.e. was causing the problems of #749.

scls19fr · 2018-08-13T20:49:33Z

Ok I will remove my .julia folder to double check

scls19fr · 2018-08-13T21:20:35Z

Sorry to say that... but

C:\Users\Admin-pc\.julia\dev\Documenter>julia
               _
   _       _ _(_)_     |  A fresh approach to technical computing
  (_)     | (_) (_)    |  Documentation: https://docs.julialang.org
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 0.7.0 (2018-08-08 06:46 UTC)
 _/ |\__'_|_|_|\__'_|  |  Official http://julialang.org/ release
|__/                   |  x86_64-w64-mingw32

(v0.7) pkg> add Documenter#fe/fix-parsing
  Updating registry at `C:\Users\Admin-pc\.julia\registries\General`
  Updating git-repo `https://github.com/JuliaRegistries/General.git`
  Updating git-repo `https://github.com/JuliaDocs/Documenter.jl.git`
 Resolving package versions...
  Updating `C:\Users\Admin-pc\.julia\environments\v0.7\Project.toml`
 [no changes]
  Updating `C:\Users\Admin-pc\.julia\environments\v0.7\Manifest.toml`
 [no changes]

(v0.7) pkg> up
  Updating registry at `C:\Users\Admin-pc\.julia\registries\General`
  Updating git-repo `https://github.com/JuliaRegistries/General.git`
  Updating git-repo `https://github.com/JuliaDocs/Documenter.jl.git`
 Resolving package versions...
  Updating `C:\Users\Admin-pc\.julia\environments\v0.7\Project.toml`
 [no changes]
  Updating `C:\Users\Admin-pc\.julia\environments\v0.7\Manifest.toml`
 [no changes]

julia>

C:\Users\Admin-pc\.julia\dev\Documenter>cd ../DataStructures

C:\Users\Admin-pc\.julia\dev\DataStructures>cd docs

C:\Users\Admin-pc\.julia\dev\DataStructures\docs>julia make.jl
Documenter: setting up build directory.
ERROR: LoadError: IOError: stat: operation not permitted (EPERM)
Stacktrace:
 [1] stat(::String) at .\stat.jl:68
 [2] isdir at .\stat.jl:303 [inlined]
 [3] runner(::Type{Documenter.Builder.SetupBuildDirectory}, ::Documenter.Documents.Document) at C:\Users\Admin-pc\.julia\packages\Documenter\wDjsI\src\Builder.jl:91
 [4] dispatch(::Type{Documenter.Builder.DocumentPipeline}, ::Documenter.Documents.Document) at C:\Users\Admin-pc\.julia\packages\Documenter\wDjsI\src\Utilities\Selectors.jl:168
 [5] #2 at C:\Users\Admin-pc\.julia\packages\Documenter\wDjsI\src\Documenter.jl:206 [inlined]
 [6] cd(::getfield(Documenter, Symbol("##2#3")){Documenter.Documents.Document}, ::String) at .\file.jl:85
 [7] #makedocs#1 at C:\Users\Admin-pc\.julia\packages\Documenter\wDjsI\src\Documenter.jl:205 [inlined]
 [8] (::getfield(Documenter, Symbol("#kw##makedocs")))(::NamedTuple{(:format, :sitename, :pages, :modules),Tuple{Symbol,String,Array{String,1},Array{Module,1}}}, ::typeof(makedocs)) at .\none:0
 [9] top-level scope at none:0
 [10] include at .\boot.jl:317 [inlined]
 [11] include_relative(::Module, ::String) at .\loading.jl:1038
 [12] include(::Module, ::String) at .\sysimg.jl:29
 [13] exec_options(::Base.JLOptions) at .\client.jl:239
 [14] _start() at .\client.jl:432
in expression starting at C:\Users\Admin-pc\.julia\dev\DataStructures\docs\make.jl:5

C:\Users\Admin-pc\.julia\dev\DataStructures\docs>julia make.jl
Documenter: setting up build directory.
Documenter: expanding markdown templates.
 !! No docs found for 'find(m::SortedDict, k_)'. [src\sorted_containers.md]
 !! No docs found for 'similar(sc::SortedDict)'. [src\sorted_containers.md]
 !! No docs found for 'get(sd::SortedDict,k,v)'. [src\sorted_containers.md]
 !! No docs found for 'get!(sd::SortedDict,k,v)'. [src\sorted_containers.md]
WARNING: importing deprecated binding Base.Associative into DataStructures.
WARNING: Base.Associative is deprecated, use AbstractDict instead.
  likely near C:\Users\Admin-pc\.julia\dev\DataStructures\docs\make.jl:5
WARNING: Base.Associative is deprecated, use AbstractDict instead.
  likely near C:\Users\Admin-pc\.julia\dev\DataStructures\docs\make.jl:5
Documenter: building cross-references.
Documenter: running document checks.
 > checking for missing docstrings.
 !! 96 docstrings potentially missing:
 > running doctests.
 > checking footnote links.
Documenter: populating indices.
Documenter: rendering document.
Documenter: skipping docs deployment.
  You can set DOCUMENTER_DEBUG to "true" in Travis to see more information.

Browsing to file:///C:/Users/Admin-pc/.julia/dev/DataStructures/docs/build/default_dict.html and
I still have

julia> dd = DefaultDict{AbstractString, Vector{Int}}(Vector{Int})
DefaultDict{AbstractString,Array{Int64,1},DataType} with 0 entries

julia> p
┌ Warning: Deprecated syntax `underscores as an rvalue`.
└ @ none:0
ERROR: UndefVarError:  not defined

Maybe I did something wrong

mortenpi

LGTM. Should definitely be CRLF nor LFCR if we're looking for windows line endings.

fredrikekre · 2018-08-13T21:28:21Z

@scls19fr do you mean that the previous version of this PR worked? It looks like you get different errors now.

scls19fr · 2018-08-13T21:41:52Z

This is because it's not the same part of page (with the minimal md file I mentioned #790 (comment) I get)

julia> dd = DefaultDict{AbstractString, Vector{Int}}(Vector{Int})
DefaultDict{AbstractString,Array{Int64,1},DataType} with 0 entries

julia> p
┌ Warning: Deprecated syntax `underscores as an rvalue`.
└ @ none:0
ERROR: UndefVarError:  not defined

julia> ush!(dd["A"], 1)
ERROR: UndefVarError: ush! not defined

julia> p
┌ Warning: Deprecated syntax `underscores as an rvalue`.
└ @ none:0
ERROR: UndefVarError:  not defined

julia> ush!(dd["B"], 2)
ERROR: UndefVarError: ush! not defined

julia> d
┌ Warning: Deprecated syntax `underscores as an rvalue`.
└ @ none:0
ERROR: UndefVarError:  not defined

julia> d
ERROR: UndefVarError: d not defined

I think the previous PR was working correctly.
Did you squash commits? Can remember what you changed

Datseris · 2018-08-13T21:43:31Z

This may sound stupid, but I recall from Slack that add Package#somebranch always adds the master branch and one has to check out manually via git....?

KristofferC · 2018-08-13T21:44:53Z

add Package#branch should get the correct branch, otherwise that is a bug.

scls19fr · 2018-08-13T21:46:51Z

This is probably the kind of issue I'm also facing.

C:\Users\Admin-pc\.julia\dev\Documenter>julia
               _
   _       _ _(_)_     |  A fresh approach to technical computing
  (_)     | (_) (_)    |  Documentation: https://docs.julialang.org
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 0.7.0 (2018-08-08 06:46 UTC)
 _/ |\__'_|_|_|\__'_|  |  Official http://julialang.org/ release
|__/                   |  x86_64-w64-mingw32

(v0.7) pkg> add Documenter#fe/fix-parsing
  Updating registry at `C:\Users\Admin-pc\.julia\registries\General`
  Updating git-repo `https://github.com/JuliaRegistries/General.git`
  Updating git-repo `https://github.com/JuliaDocs/Documenter.jl.git`
 Resolving package versions...
  Updating `C:\Users\Admin-pc\.julia\environments\v0.7\Project.toml`
 [no changes]
  Updating `C:\Users\Admin-pc\.julia\environments\v0.7\Manifest.toml`
 [no changes]

(v0.7) pkg> ^C

julia> ^C

julia>

C:\Users\Admin-pc\.julia\dev\Documenter>git branch
* master

fredrikekre · 2018-08-13T21:48:10Z

added branches does not end up in dev

Datseris · 2018-08-13T21:49:20Z

add Package#branch should get the correct branch, otherwise that is a bug.

It does, just tested it as well. Sorry for the off-topic comment!

scls19fr · 2018-08-13T21:52:38Z

How can I check that I'm running the "right" Documenter (ie from fe/fix-parsing branch)?

fredrikekre · 2018-08-13T22:01:39Z

Ok, found a simple testcase that still fails on this branch. Will ping you when I have a new version that you can try.

fredrikekre · 2018-08-13T23:47:51Z

Okay, @Datseris and @scls19fr feel free to test the new version.

scls19fr · 2018-08-14T06:49:31Z

After you merged to master, I did

Pkg.rm("Documenter")
Pkg.develop("Documenter")

but I still facing #790

KristofferC · 2018-08-14T07:16:40Z

You might have to pull the newest version manually, there is a bug where develop fails to get the latest version.

Datseris · 2018-08-14T07:29:10Z

I can still confirm that on the current master this works for me.

scls19fr · 2018-08-14T07:42:53Z

Under Mac OS X now

$ cd ~/.julia/dev/Documenter
$ git pull
$ cd ~/.julia/dev/DataStructures/docs
$ julia make.jl

Minimal md file mentioned in #790 (comment) saved with CRLF or LF as EOL still render badly

julia> dd = DefaultDict{AbstractString, Vector{Int}}(Vector{Int})
DefaultDict{AbstractString,Array{Int64,1},DataType} with 0 entries

┌ Warning: Deprecated syntax `underscores as an rvalue`.
└ @ none:0

julia> push!(dd["A"], 1)
1-element Array{Int64,1}:
 1

┌ Warning: Deprecated syntax `underscores as an rvalue`.
└ @ none:0

julia> push!(dd["B"], 2)
1-element Array{Int64,1}:
 2

┌ Warning: Deprecated syntax `underscores as an rvalue`.
└ @ none:0

julia> dd
DefaultDict{AbstractString,Array{Int64,1},DataType} with 2 entries:
  "B" => [2]
  "A" => [1]

scls19fr · 2018-08-14T08:00:54Z

Trying with an even simpler file

# Dict

```@repl
dd = Dict{AbstractString, Int}()

dd["A"] = 1

dd

(CRLF or LF)

rendered as

julia> dd = Dict{AbstractString, Int}()
Dict{AbstractString,Int64} with 0 entries

┌ Warning: Deprecated syntax `underscores as an rvalue`.
└ @ none:0

julia> dd["A"] = 1
1

┌ Warning: Deprecated syntax `underscores as an rvalue`.
└ @ none:0

julia> dd
Dict{AbstractString,Int64} with 1 entry:
  "A" => 1

(cherry picked from commit fe8a644)

fredrikekre requested a review from mortenpi August 13, 2018 13:11

fredrikekre force-pushed the fe/fix-parsing branch from 6f451f3 to 1304062 Compare August 13, 2018 13:13

fredrikekre commented Aug 13, 2018

View reviewed changes

fredrikekre force-pushed the fe/fix-parsing branch from 1304062 to 202deb1 Compare August 13, 2018 14:13

mortenpi added the Type: Bug label Aug 13, 2018

mortenpi added this to the 0.19.5 milestone Aug 13, 2018

mortenpi added the Status: Backport Needed label Aug 13, 2018

mortenpi approved these changes Aug 13, 2018

View reviewed changes

fredrikekre force-pushed the fe/fix-parsing branch from 202deb1 to eb140c6 Compare August 13, 2018 23:45

fredrikekre force-pushed the fe/fix-parsing branch from eb140c6 to 9637d13 Compare August 14, 2018 00:13

Fix parsing of double CRLF newline.

317d20e

fredrikekre force-pushed the fe/fix-parsing branch from 9637d13 to 317d20e Compare August 14, 2018 00:17

fredrikekre merged commit fe8a644 into master Aug 14, 2018

fredrikekre deleted the fe/fix-parsing branch August 14, 2018 04:44

mortenpi pushed a commit that referenced this pull request Aug 15, 2018

Fix parsing of double CRLF newline. (#791)

ab09093

(cherry picked from commit fe8a644)

This was referenced Aug 15, 2018

Backports for 0.19.5 #797

Merged

Documenter.jl does not work on Windows 10 and Julia 0.7 #771

Closed

mortenpi removed the Status: Backport Needed label Aug 16, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make sure we have LF line endings only. #791

Make sure we have LF line endings only. #791

fredrikekre commented Aug 13, 2018

fredrikekre commented Aug 13, 2018

fredrikekre commented Aug 13, 2018

Datseris commented Aug 13, 2018

scls19fr commented Aug 13, 2018

fredrikekre Aug 13, 2018

KristofferC Aug 13, 2018

fredrikekre Aug 13, 2018

fredrikekre Aug 13, 2018

fredrikekre commented Aug 13, 2018

Datseris commented Aug 13, 2018

fredrikekre Aug 13, 2018

fredrikekre commented Aug 13, 2018

scls19fr commented Aug 13, 2018

Datseris commented Aug 13, 2018

scls19fr commented Aug 13, 2018 •

edited

scls19fr commented Aug 13, 2018

mortenpi left a comment

fredrikekre commented Aug 13, 2018

scls19fr commented Aug 13, 2018

Datseris commented Aug 13, 2018

KristofferC commented Aug 13, 2018

scls19fr commented Aug 13, 2018

fredrikekre commented Aug 13, 2018

Datseris commented Aug 13, 2018 •

edited

scls19fr commented Aug 13, 2018 •

edited

fredrikekre commented Aug 13, 2018

fredrikekre commented Aug 13, 2018

scls19fr commented Aug 14, 2018 •

edited

KristofferC commented Aug 14, 2018

Datseris commented Aug 14, 2018

scls19fr commented Aug 14, 2018

scls19fr commented Aug 14, 2018

Make sure we have LF line endings only. #791

Make sure we have LF line endings only. #791

Conversation

fredrikekre commented Aug 13, 2018

fredrikekre commented Aug 13, 2018

fredrikekre commented Aug 13, 2018

Datseris commented Aug 13, 2018

scls19fr commented Aug 13, 2018

fredrikekre Aug 13, 2018

Choose a reason for hiding this comment

KristofferC Aug 13, 2018

Choose a reason for hiding this comment

fredrikekre Aug 13, 2018

Choose a reason for hiding this comment

fredrikekre Aug 13, 2018

Choose a reason for hiding this comment

fredrikekre commented Aug 13, 2018

Datseris commented Aug 13, 2018

fredrikekre Aug 13, 2018

Choose a reason for hiding this comment

fredrikekre commented Aug 13, 2018

scls19fr commented Aug 13, 2018

Datseris commented Aug 13, 2018

scls19fr commented Aug 13, 2018 • edited

scls19fr commented Aug 13, 2018

mortenpi left a comment

Choose a reason for hiding this comment

fredrikekre commented Aug 13, 2018

scls19fr commented Aug 13, 2018

Datseris commented Aug 13, 2018

KristofferC commented Aug 13, 2018

scls19fr commented Aug 13, 2018

fredrikekre commented Aug 13, 2018

Datseris commented Aug 13, 2018 • edited

scls19fr commented Aug 13, 2018 • edited

fredrikekre commented Aug 13, 2018

fredrikekre commented Aug 13, 2018

scls19fr commented Aug 14, 2018 • edited

KristofferC commented Aug 14, 2018

Datseris commented Aug 14, 2018

scls19fr commented Aug 14, 2018

scls19fr commented Aug 14, 2018

scls19fr commented Aug 13, 2018 •

edited

Datseris commented Aug 13, 2018 •

edited

scls19fr commented Aug 13, 2018 •

edited

scls19fr commented Aug 14, 2018 •

edited