search

mozilla / assay

A Firefox Addons review tool disguised as a VSCode extension
Mozilla Public License 2.0
9 stars 3 forks source link

More Informative External Diff Tool Error Messages #100

Open chrstinalin opened 4 months ago

chrstinalin commented 4 months ago

There's a debatably uninformative error message when the command doesn't exist (image, ENOENT) or is used incorrectly (No error message at all).

image

From #99

willdurand commented 4 months ago

I changed the label because while the error message might not be clear (arguably ENOENT is pretty common for developers), there is still an error message. We should make it better and more clear, though.

chrstinalin commented 4 months ago

@willdurand I figured, but the silent error is the one that's more worrisome to me

willdurand commented 4 months ago

Ha, I see. I am wondering if the silent error is because there is no validation on the setting. Maybe we could force a non-empty value for this setting?

chrstinalin commented 4 months ago

@willdurand In the case I tested, I used git as the diff tool command. Since git does not throw an error (responds with "is not a git command"), no error message is shown on Assay's side (openInDiffTool() indiffController.ts`).

There's probably an argument to be made here that it's no longer Assay's responsibility at that point, but I think an information message showing "success" would be helpful so that the user at least knows something happened.

willdurand commented 4 months ago

Fair enough, though it's very low priority since we added a default value that reviewers will likely not change anyway.