WIP: Make error message for conflict contain suggestions of a couple resolution paths #13

Closed

NullVoxPopuli wants to merge 1 commit from add-error-context into main

NullVoxPopuli commented

2023-04-16 14:57:34 +02:00

(Migrated from github.com)

I started here: https://discuss.codemirror.net/t/how-can-i-help-make-grammar-authoring-less-terse-unhelpful/6286/1

My hope is to learn Lezer by helping the errors reported be less terse, and provide a little more guidance.

What has been done in this PR is focused around the overlapping tokens message, and the suggestions added may not be complete, but it's a start! We can iterate and improve the messaging together.

As an aside, I noticed there was no CONTRIBUTING.md, dev section of the README, or .github/ci.yml, so I'm not sure how to run the tests, I kept getting errors locally.

Update!

npm run build main && npm run test passes!

so, that means there is no test testing for an exact error message

I started here: https://discuss.codemirror.net/t/how-can-i-help-make-grammar-authoring-less-terse-unhelpful/6286/1 My hope is to learn Lezer by helping the errors reported be less terse, and provide a little more guidance. What has been done in this PR is focused around the overlapping tokens message, and the suggestions added may not be complete, but it's a start! We can iterate and improve the messaging together. --------------------------- As an aside, I noticed there was no CONTRIBUTING.md, dev section of the README, or .github/ci.yml, so I'm not sure how to run the tests, I kept getting errors locally. -------------------- Update! `npm run build main && npm run test` passes! _so_, that means there is no test testing for an exact error message

marijnh commented

2023-04-16 15:17:18 +02:00

(Migrated from github.com)

I don't think recommending @precedence in this error message is a good idea, since that is often only going to get people deeper into the hole. In the situation you mentioned in the forum thread, for example, unless I entirely misunderstand what the grammar is doing, what you need to do is make sure Text and identifier don't appear in the same context in the grammar. Giving one of them precedence over the other is just going to break things even worse.

And that's a general thing about adding suggestions to error messages—they cannot change the fact that the user has to understand what they are doing. And if they aren't done in a really careful way, they will only steer users in the wrong direction.

I don't think recommending `@precedence` in this error message is a good idea, since that is often only going to get people deeper into the hole. In the situation you mentioned in the forum thread, for example, unless I entirely misunderstand what the grammar is doing, what you need to do is make sure `Text` and `identifier` don't appear in the same context in the grammar. Giving one of them precedence over the other is just going to break things even worse. And that's a general thing about adding suggestions to error messages—they cannot change the fact that the user has to understand what they are doing. And if they aren't done in a really careful way, they will only steer users in the wrong direction.

NullVoxPopuli commented

2023-04-16 15:21:35 +02:00

(Migrated from github.com)

since that is often only going to get people deeper into the hole.

Then the error message should steer folks out of the hole, and recommend not using precedence, and provide a way out

> since that is often only going to get people deeper into the hole. Then the error message should steer folks out of the hole, and recommend _not_ using precedence, and provide a way out

Pull request closed

This pull request cannot be reopened because the branch was deleted.

No reviewers

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

lezer/generator!13

No description provided.

Rows
Columns