Skip to content

Support multi-file error enrichment using SourceFile#266

Merged
KyrylR merged 1 commit intoBlockstreamResearch:dev/importsfrom
LesterEvSe:feature/better-error-handler
Apr 1, 2026
Merged

Support multi-file error enrichment using SourceFile#266
KyrylR merged 1 commit intoBlockstreamResearch:dev/importsfrom
LesterEvSe:feature/better-error-handler

Conversation

@LesterEvSe
Copy link
Copy Markdown
Collaborator

@LesterEvSe LesterEvSe commented Apr 1, 2026

Since we will support modules, we need to know exactly where the error happened. Also, for single-file programs, nothing has changed, and the errors will be displayed the same way.

@LesterEvSe LesterEvSe requested a review from delta1 as a code owner April 1, 2026 12:47
@LesterEvSe LesterEvSe changed the base branch from master to dev/imports April 1, 2026 12:47
@LesterEvSe LesterEvSe changed the title Feature/better error handler Support multi-file error enrichment using SourceFile Apr 1, 2026
@LesterEvSe LesterEvSe requested a review from KyrylR April 1, 2026 12:49
@LesterEvSe LesterEvSe self-assigned this Apr 1, 2026
@LesterEvSe LesterEvSe added the enhancement New feature or request label Apr 1, 2026
src/error.rs Outdated
}
}

pub fn with_file(self, file: Arc<str>) -> Self {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub fn with_file(self, file: Arc<str>) -> Self {
pub fn with_file(self, program_content: Arc<str>) -> Self {

src/error.rs Outdated
Comment on lines 224 to 229
pub fn file(&self) -> Option<Arc<str>> {
match &self.error_context {
Some(ErrorContext::Content(file)) => Some(Arc::clone(file)),
_ => None,
}
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add rust doc

src/error.rs Outdated
write!(f, "{:^<width$} ", "", width = underline_length)?;
write!(f, "{}", self.error)

/*
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove the comment

@LesterEvSe LesterEvSe force-pushed the feature/better-error-handler branch from 80aa418 to 4084255 Compare April 1, 2026 13:15
@KyrylR
Copy link
Copy Markdown
Collaborator

KyrylR commented Apr 1, 2026

To avoid ambiguity with RichError, maybe it is better to have the following API shape:

  • SourceFile { content: Arc<str>, name: Option<Arc<Path>> }
  • RichError { error, span, source: Option<SourceFile> }
  • parse_with_errors(source: &SourceFile, handler: &mut ErrorCollector)
  • ErrorCollector::for_source(source.clone()) plus plain push/extend

Then clean up the API surface:

  • Deprecate file(). It is ambiguous now (update the CHANGELOG.md)
  • Replace file() it with explicit accessors like source_text(), source_name(), and maybe source()

@LesterEvSe LesterEvSe force-pushed the feature/better-error-handler branch from 4084255 to 2dbadee Compare April 1, 2026 14:27
@LesterEvSe
Copy link
Copy Markdown
Collaborator Author

LesterEvSe commented Apr 1, 2026

  • parse_with_errors(source: &SourceFile, handler: &mut ErrorCollector)

I did not change it because doing so could break the entire parser logic

@LesterEvSe LesterEvSe force-pushed the feature/better-error-handler branch from 2dbadee to 816b746 Compare April 1, 2026 14:41
@LesterEvSe
Copy link
Copy Markdown
Collaborator Author

  • ErrorCollector::for_source(source.clone()) plus plain push/extend

The ErrorCollector::for_source(source.clone()) function is not needed because it is handled automatically inside the RichError::fmt(...) function

@LesterEvSe LesterEvSe force-pushed the feature/better-error-handler branch from 816b746 to d8a81f3 Compare April 1, 2026 14:45
@LesterEvSe LesterEvSe requested a review from KyrylR April 1, 2026 14:46
@LesterEvSe LesterEvSe force-pushed the feature/better-error-handler branch from d8a81f3 to 026c0ff Compare April 1, 2026 14:54
@KyrylR KyrylR merged commit 026c0ff into BlockstreamResearch:dev/imports Apr 1, 2026
11 checks passed
@KyrylR KyrylR mentioned this pull request Apr 1, 2026
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants