From 4ac765c9c9140c4645c34b93acfbb6b96287791e Mon Sep 17 00:00:00 2001 From: Claire Foster Date: Sat, 28 Oct 2023 15:47:10 +1000 Subject: [PATCH] Fix diagnostics printing when `pwd()` doesn't exist Computing the file URL for pretty printing using `abspath` can fail when the current working directory doesn't exist. This is a quick fix for this issue (which is incredibly confusing/disruptive if the user does manage to enter a nonexistant working directory!) A more complete fix would avoid ever looking at the working directory when printing diagnostics, instead requiring the caller to pass in a richer definition of the "location of source code" than the mere file name as a string. However getting something sensible working there is (a) breaking and (b) unclear on may details. So just patching this up quickly seems good for now. --- src/diagnostics.jl | 32 ++++++++++++++++++++++++-------- test/diagnostics.jl | 21 +++++++++++++++++++++ 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/src/diagnostics.jl b/src/diagnostics.jl index c84fa0ac..9a5ea961 100644 --- a/src/diagnostics.jl +++ b/src/diagnostics.jl @@ -44,13 +44,26 @@ Base.range(d::Diagnostic) = first_byte(d):last_byte(d) # Make relative path into a file URL function _file_url(filename) - @static if Sys.iswindows() - # TODO: Test this with windows terminal - path = replace(abspath(filename), '\\'=>'/') - else - path = abspath(filename) + try + @static if Sys.iswindows() + # TODO: Test this with windows terminal + path = replace(abspath(filename), '\\'=>'/') + else + path = abspath(filename) + end + return "file://$(path)" + catch exc + # abspath may fail if working directory doesn't exist + # TODO: It seems rather non-ideal to have the behavior here depend on + # the state of the local filesystem. And yet links in diagnostics seem + # useful. + # + # Ideally it'd be up to the caller to provide some notion of the + # "absolute location" of the source code resource when SourceFile is + # constructed. This is often not related to the local filesystem - it + # could be in memory, a fragment embedded in another file, etc etc. + return nothing end - "file://$(path)" end function show_diagnostic(io::IO, diagnostic::Diagnostic, source::SourceFile) @@ -64,8 +77,11 @@ function show_diagnostic(io::IO, diagnostic::Diagnostic, source::SourceFile) file_href = nothing if !isnothing(filename) locstr = "$filename:$linecol" - if !startswith(filename, "REPL[") - file_href = _file_url(filename)*"#$linecol" + if !startswith(filename, "REPL[") && get(io, :color, false) + url = _file_url(filename) + if !isnothing(url) + file_href = url*"#$linecol" + end end else locstr = "line $linecol" diff --git a/test/diagnostics.jl b/test/diagnostics.jl index ea2feb37..66fe4fda 100644 --- a/test/diagnostics.jl +++ b/test/diagnostics.jl @@ -218,4 +218,25 @@ end # Error @ line 1:8 a -- b -- c # └┘ ── invalid operator""" + + stream = JuliaSyntax.ParseStream("a -- b") + JuliaSyntax.parse!(stream) + fname = "test.jl" + sf = SourceFile(stream, filename=fname) + url = JuliaSyntax._file_url(fname) + @test sprint(JuliaSyntax.show_diagnostics, stream.diagnostics, sf, + context=:color=>true) == """ + \e[90m# Error @ \e[0;0m\e]8;;$url#1:3\e\\\e[90mtest.jl:1:3\e[0;0m\e]8;;\e\\ + a \e[48;2;120;70;70m--\e[0;0m b + \e[90m# └┘ ── \e[0;0m\e[91minvalid operator\e[0;0m""" + + if Sys.isunix() + mktempdir() do tempdirname + cd(tempdirname) do + rm(tempdirname) + # Test _file_url doesn't fail with nonexistant directories + @test isnothing(JuliaSyntax._file_url(joinpath("__nonexistant__", "test.jl"))) + end + end + end end