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

Support backtrace #78

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Support backtrace #78

wants to merge 2 commits into from

Conversation

oovm
Copy link

@oovm oovm commented Jun 2, 2023

No description provided.

@zesterer
Copy link
Owner

zesterer commented Jun 2, 2023

Hi, thanks for the PR!

I think some aspects of this PR are a bit too specific for ariadne. Ariadne is intended to be a diagnostic renderer (i.e: something that takes error message text, spans, and source and converts it to human-readable output) with a specific bias toward compiler output.

For example, CausedByFrame::io_error concerns me because it's very prescriptive about how an IO error should be turned into text: this feels like it's better suited to a more Rust-oriented crate like anyhow (and kin) or some glue code between the two (an example of such a thing that I briefly worked on is here).

Additionally, the use of line/column in CausedByFrame is another point of concern for me: ariadne is intended to ingest spans from the original source, not to leave converting between byte spans and line/column and such to the end user.

I worry that a lot of this backtrace logic doesn't fit well within the existing crate scope, especially as it seems like it can't interact with label rendering (you can't, say, point to the location of a backtrace segment in the original source).

My instinct would be that this is better suited to an extension crate on top of ariadne, although it's true that there are systems that first need work in ariadne to facilitate that. What do you think?

@zesterer zesterer mentioned this pull request Jun 2, 2023
@oovm
Copy link
Author

oovm commented Jun 2, 2023

Report as io error is indeed an abuse.

The main use should be to output the location of the call site, what information to give is up to the user.

As for whether the interaction depends on the terminal used by the user and the target platform, I really don't know any mechanism that can interact in all the default mid-range.

In fact there is a problem on my terminal, one part can be clicked, the other part can't.

QQ截图20230602204739

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants