This is an archive of the discontinued Mercurial Phabricator instance.

rust: use HgError in RevlogError and Vfs
ClosedPublic

Authored by SimonSapin on Jan 27 2021, 4:13 PM.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

SimonSapin created this revision.Jan 27 2021, 4:13 PM

These changes start mixing user output with core logic, which didn't use to happen. Since we're not doing anything particular with those errors, I'm fine with that change (I makes the code a lot simpler and is a welcome change), but it's possible we'll have to re-introduce finer-grained errors if need be. The matter of i18n remains completely unsolved, but that's for another time.

rust/hg-core/src/repo.rs
73

I know this is makes for better errors, but the API is kind of clunky. Fortunately my solution would have been to encapsulate this... which is what the VFS is for.

Alphare accepted this revision.Jan 29 2021, 8:51 AM

mixing user output with core logic

How do you mean?

rust/hg-core/src/repo.rs
73

Why clunky? Because it makes callers handle variants of HgError that are never returned? We could introduce a IoError with context, as an independent type for just the corresponding variant of HgError.

Alphare added inline comments.Jan 29 2021, 12:29 PM
rust/hg-core/src/repo.rs
73

Oh no, simply because for_file(&path) kind of feels redundant, as if it should be in fs::read in the first place, but it's not a big deal, more of a remark.

This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.