-
Notifications
You must be signed in to change notification settings - Fork 217
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
More Descriptive Errors (Part 1/2) #298
Conversation
- Trying to preserve the pattern where error data-types and constructor are prefixed with `Err...` which identifies them right away as errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @KtorZ!
I have to admit, I'm not yet convinced that it's a good idea to use the QuasiText
quasiquoter here. I can think of a few disadvantages:
-
It silently inserts a
\n
at the end of each line. While it's true that our code is wrapped to a width of 80 characters, this particular wrapping might not be appropriate for the user's view port. (Error messages can of course be viewed in different contexts: perhaps we should let the user's view port determine the wrapping?) -
It doesn't play nicely with our 4-space indentation rule. If we do indent, then extra spaces are inserted in the resultant string. If we don't indent, then it breaks the nested structure of our code and makes the code harder to read (in my opinion).
-
It doesn't play nicely with syntax highlighting. Most Haskell syntax highlighters can highlight string literals in a different colour (e.g., bright red), but that won't work with this quasi-quoter. In fact, certain words such as "data", if included in the string, could mistakenly be highlighted as keywords. This makes it harder to visually differentiate strings from code.
Compare the following two snippets:
instance LiftHandler ErrCoinSelection where
handler = \case
ErrNotEnoughMoney utxo payment ->
apiError err403 NotEnoughMoney [embed|
I can't accomodate such payment because there's not enough available UTxO in the
wallet. The total UTxO sums up to $(utxo) Lovelace, but I need $(payment)
Lovelace (fee included) in order to proceed with the payment.|]
ErrUtxoNotEnoughFragmented nUtxo nOuts ->
apiError err403 UtxoNotEnoughFragmented [embed|
There's a restriction in the way I can construct transactions: I do not re-use a
same UTxO for different outputs. Here, I only have $(nUtxo) available but there
are $(nOuts) outputs.|]
ErrMaximumInputsReached n ->
apiError err403 TransactionIsTooBig [embed|
I had to select as many as $(n) inputs to construct the requested transaction.
This would create a transaction that is too big and would be rejected by a core
node. Try sending a smaller amount.|]
with:
instance LiftHandler ErrCoinSelection where
handler = \case
ErrNotEnoughMoney utxo payment ->
apiError err403 NotEnoughMoney $ mconcat
[ "I can't accommodate this payment because there's not enough "
, "available UTxO in the wallet. The total UTxO sums up to "
, toText utxo, " Lovelace, but I need ", toText payment
, " Lovelace (fee included) in order to proceed with the "
, "payment." ]
ErrUtxoNotEnoughFragmented nUtxo nOuts ->
apiError err403 UtxoNotEnoughFragmented $ mconcat
[ "There's a restriction in the way I can construct "
, "transactions: I do not re-use a same UTxO for different "
, "outputs. Here, I only have ", toText nUtxo, " available but "
, "there are ", toText nOuts, " outputs." ]
ErrMaximumInputsReached n ->
apiError err403 TransactionIsTooBig $ mconcat
[ "I had to select as many as ", toText n, " inputs to "
, "construct the requested transaction. This would create a "
, "transaction that is too big and would be rejected by a core "
, "node. Try sending a smaller amount." ]
In the above pair of examples, there's not much difference in vertical space usage, but the latter has several advantages:
- There are no
\n
s silently inserted. - It's indented according to the logical structure of the code.
- Syntax highlighting works as normal.
Perhaps I'm missing something?
Hi @KtorZ Regarding English usage, it seems that we have a mixture of two different styles:
I think it would be best to pick one of these styles, and then use it consistently throughout our messages. As for which style to choose: I would favour sticking to the impersonal style, as it seems more professional (and consistent with other tools that are out there). What do you think? |
Agreed with your first point. Note that the resulting messages don't contain any About the tone, I have to say that I kinda like the impersonal style. That's also what the Elm compiler uses and I find it delightful while developing with Elm. It kinda avoid some friction with the system, but I suppose that's really educational / cultural here. I 100% agree however about sticking to one style and be consistent with it. |
726f969
to
04ec30f
Compare
\following output address was not found: " <> pretty addr | ||
ErrSignTx (ErrKeyNotFoundForAddress addr) -> | ||
apiError err403 KeyNotFoundForAddress $ mconcat | ||
[ "I couldn't sign the given transaction: I didn't found the " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose: I didn't find or I haven't found
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm 👍 (except one small typo
)
…friendly escaping issues
6223bce
to
1728c77
Compare
I just saw your updated commits! At least on my editor, it looks much much easier to read. I agree it would be great to have a nicer quasi-quoter that does the Right Thing™ w.r.t. ignoring indentation and newlines. (Perhaps a minor side project for us when there's a quiet period?)
Do you mean the anthropomorphic style, where the wallet refers to itself in the first person? (I have to admit that I just made up these terms, so feel free to suggest something better!)
Interesting, I wasn't aware of that! I've just taken a look at the Compiler Errors for Humans essay. The error messages seem very nicely written, and consistent. Very convincing!
💯 As per your original request, I'll take another look at the language content of the messages themselves. |
Whoops. That's what I meant indeed ^^" ...
And this was only the version 0.5 I believe. With 0.19, it's even better. I wish all compilers would be like that <3 |
Issue Number
#260
Overview
Comments
Waiting for @jonathanknowles to control the English here 😛
A few examples: