Page MenuHomePhabricator

errors: introduce InputError and use it from commands and cmdutil
Needs ReviewPublic

Authored by martinvonz on Wed, Oct 7, 2:10 AM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Summary

This patch introduces a InputError class and replaces many uses of
error.Abort by it in commands and cmdutil. This is a part of
https://www.mercurial-scm.org/wiki/ErrorCategoriesPlan. There will
later be a different class for state errors (to raise e.g. when
there's an unfinished operation). It's not always clear when one
should report an input error and when it should be a state error. We
can always adjust later if I got something wrong in this patch (but
feel free to point out any you notice now).

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

martinvonz created this revision.Wed, Oct 7, 2:10 AM
martinvonz updated this revision to Diff 23073.Wed, Oct 7, 2:17 AM
pulkit added a subscriber: pulkit.Wed, Oct 7, 7:53 AM

The plan page mentions InputError instead of UserError which I think is more fine-grained and better suited in some cases. Reading the relevant email thread titled Proposal for cleaning up error reporting, it seems that we have not finalized these names.

The plan page mentions InputError instead of UserError which I think is more fine-grained and better suited in some cases.

I felt like InputError made it sounds like it was specifically for errors in input when hg was asking for input, not including inputs given on the command line. I'm fine with using the proposed InputError, though. I'll wait a bit to hear other people's opinions first.

Reading the relevant email thread titled Proposal for cleaning up error reporting, it seems that we have not finalized these names.

Sorry, I clearly should have reviewed that thread first. I thought the thread ended with Augie's message about forcing opinions from non-Googlers by sending a patch. I also remember saying that I was skeptical about changing error statuses. Apparently both those memories were incorrect :) However, the thread I reviewed just now doesn't seem to talk about the names at all. Were there several threads?

martinvonz retitled this revision from errors: introduce UserError and use it from commands and cmdutil to errors: introduce InputError and use it from commands and cmdutil.Mon, Oct 12, 2:29 PM
martinvonz edited the summary of this revision. (Show Details)
martinvonz updated this revision to Diff 23177.

I've renamed the exception to InputError to unblock this. Let me know if there was anything else blocking. I wasn't sure if you wanted to see more discussion about the name on thread before we decide.

I've renamed the exception to InputError to unblock this. Let me know if there was anything else blocking. I wasn't sure if you wanted to see more discussion about the name on thread before we decide.

Thank you. I am thinking to get back to this (queue this maybe) once we cut the upcoming RC (after 3 days). I agree that there are no BC changes but doing this will make things easier as then we can try to have all these changes in one release. Doesn't yield much but sounds like a good idea to me. Let me know if you prefer it to be queued before.

I've renamed the exception to InputError to unblock this. Let me know if there was anything else blocking. I wasn't sure if you wanted to see more discussion about the name on thread before we decide.

Thank you. I am thinking to get back to this (queue this maybe) once we cut the upcoming RC (after 3 days). I agree that there are no BC changes but doing this will make things easier as then we can try to have all these changes in one release. Doesn't yield much but sounds like a good idea to me. Let me know if you prefer it to be queued before.

That sounds reasonable. There's no rush with this from our side. Thanks.