YAML.jl icon indicating copy to clipboard operation
YAML.jl copied to clipboard

BUG: incorrect line numbering for windows files

Open albop opened this issue 8 years ago • 2 comments

When trying to implement a linter using YAML.jl, @ezgioz found an inconsistency in the line numbering when importing a file under windows (I think we discussed it with @sglyon a while ago). The bug can be replicated using the following code:

julia> s1 = "a: [1,2,3]\nb: [4,5,6]\nc: [7,8,9]\n"
"a: [1,2,3]\nb: [4,5,6]\nc: [7,8,9]\n"

julia> s2 = "a: [1,2,3]\r\nb: [4,5,6]\r\nc: [7,8,9]\r\n"
"a: [1,2,3]\r\nb: [4,5,6]\r\nc: [7,8,9]\r\n"

julia> yml_types = Dict{AbstractString,Function}("tag:yaml.org,2002:seq"=>((c,n)->println(n.start_mark)))
Dict{AbstractString,Function} with 1 entry:
  "tag:yaml.org,2002:seq" => #1

julia> YAML.load(s1, yml_types)
line 1, column 3
line 2, column 3
line 3, column 3
Dict{Any,Any} with 3 entries:
  "c" => nothing
  "b" => nothing
  "a" => nothing

julia> YAML.load(s2, yml_types)
line 1, column 3
line 3, column 3
line 5, column 3
Dict{Any,Any} with 3 entries:
  "c" => nothing
  "b" => nothing
  "a" => nothing

End of lines, are counted twice. Apparently, on windows, switching end-of-line conventions (by replacing \r\n by \n) fixes the problem.

I suspect the incorrect count comes from this line https://github.com/dcjones/YAML.jl/blob/master/src/scanner.jl#L128

if in(c, "\n\u0085\u2028\u2029") ||
     (c == '\r' && peek(stream.input) == '\n')
      stream.column = 0
      stream.line += 1

Replacing the condition by:

if in(c, "\n\u0085\u2028\u2029") ||
     (c == '\r' && peek(stream.input) == '\n')

seems to fix the problem, but I wouldn't swear there are no side effects since, the scanner.jl file seems to metnion the \r quite often.

albop avatar Aug 06 '17 14:08 albop

Hi @albop thanks for opening here. I don't have access to a Windows machine and don't have any expertise to offer here.

Do you think the test suite is robust enough to push forward with your proposed change if all tests pass on Windows, OSX, and Linux? If so, then I think it is probably a good idea.

sglyon avatar Aug 16 '17 15:08 sglyon

FYI, I dealt with it this way in my own code:

	try
		YAML.load_file(config_file)
	catch err
		exit_status = 1
		if Sys.iswindows()
			eol_length = 2 # YAML mis-counts lines on Windows
		else
			eol_length = 1
		end
		println("Configuration file '" * config_file * "' did not load properly: " * err.problem * " on line " * string(err.problem_mark.line ÷ eol_length))
		return(exit_status)
	end

erickb avatar Aug 20 '22 13:08 erickb