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

give better file:lineno in the log #2310

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

simonbyrne
Copy link
Contributor

@simonbyrne simonbyrne commented Oct 24, 2023

Log now prints:

└ @ Documenter ~/src/ClimaCore.jl/docs/src/api.md:210

instead of

└ @ Documenter ~/src/Documenter/src/utilities/utilities.jl:44

which makes it easier to jump to in my editor (can ctrl-click to go there)

@fredrikekre
Copy link
Member

The location should also be printed in the message itself already. Is the problem that we print a line range there and not just a single number?

@mortenpi
Copy link
Member

mortenpi commented Nov 8, 2023

I guess the question here is whether we actually want the location info from the logging macro to not match the actual macro invocation? Arguably, this might be confusing, especially if we happen to mess up the line numbers somehow (due to a bug or something). On the other hand, the links to Documenter source are pretty useless 90% of the time, and this feels like a UX improvement.

@mortenpi
Copy link
Member

mortenpi commented Nov 8, 2023

The location should also be printed in the message itself already. Is the problem that we print a line range there and not just a single number?

Since we do already print the location information for all (?) of these blocks, then maybe we should just disable the location information from the macro, if that's possible? Does _file = nothing, _line = nothing work?

@simonbyrne
Copy link
Contributor Author

The location should also be printed in the message itself already. Is the problem that we print a line range there and not just a single number?

It doesn't print the full path, which means the ctrl-click in VSCode doesn't work. I don't know about the range? Ideally it would print the actual line, not just the block, but that seems like it would be more difficult.

I guess the question here is whether we actually want the location info from the logging macro to not match the actual macro invocation?

The problem is it points to the @docerror macro, which is not exactly helpful. A small improvement would be to instead pass the line where @docerror was invoked (using __source__).

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.

3 participants