Customizing errors

[kfsone]

Currently, gocc messages are a bit obtuse for anyone not directly using gocc, and in particular they aren't in the FLC format that makes tools easy to integrate with That Person's Environment(TM). Probably more noticeable after alternating between go compiler errors and gocc lines of terror? :)

Is there a preferred way to customize ones' own error outputs, is there a plan to improve or enable improvement of errors, e.g an option to choose between errors for developers vs errors?

I guarantee you that if I show a game designer/artist

Error in S42: identifier(8,true), Pos(offset=514, line=30, column=50), expected one of: { typename true false float_literal integer_literal string_literal loc_literal

I'll spend at least an hour a day looking their typos and explaining that the mistake wasn't on line 542, 8, or 514... They're not daft, but my tool's part in their day is supposed to be as trivial to them as the desk they're leaning on, so if I don't present them with clear and direct feedback it's both a massive context switch and a complete domain switch

attributes.def:30:50: error: expected one of: { typename true false float_literal integer_literal string_literal loc_literal, got: 'ture'. [S42: identifier(8, true), offset 512]

I also like to have my "redefinition" errors contain an FLC link to the conflicting definition, I get less "where's the other one?" phone calls that way :)

[awalterschulze]

I think we would be more than happy to consider a contribution that redesigns our error reporting and/or simply changing the way we print out errors by default.

[kfsone]

Excellent -- I'll try to get something together.

[kfsone]

Had a few moments to take Error.Error() for a quick spin and try to produce a quick mock-up:

https://gist.github.com/kfsone/ba8ce89ef2227c4230f1a9b40e3fe329

In my parser:

    if _, err = parser.Parse(lexer); err != nil {
        fmt.Println(fmt.Errorf("%s:%w", filename, err).Error())
    }

lets me produce some clear errors that are automatically linked to the source document and line in vim/emacs/vs/vsc/jetbrains and probably others.

Where there is one option for the token:

C:\Users\oliver\parsedef\baddata.blueprint:2:16: error: 8:17: error: expected typename; got: "hello"

or multiple:

C:\Users\oliver\parsedef\baddata.blueprint:2:16: error: expected one of: {, typename, true, false, float_literal, integer_literal, string_literal, or loc_literal; got: =

And a self-thrown error:

BlueprintName
    : typename
    | identifier << nil, fmt.Errorf("blueprint names must start with a capital letter; got: %s", ast.AttrToString($0)) >>
    ;

produces:

C:\Users\oliver\parsedef\baddata.blueprint:8:25: error: blueprint names must start with a capital letter; got: badnews

I'd like to quote literals, e.g. "{", "true", "false", but at the call to newError() there didn't appear to be any way to distinguish between a rule name and a literal value.

Is something like this interesting? Should it be gated behind an option? Would it be better isolating it as "don't write my error handlers for me"? Did I manage to avoid trampling every guideline you guys have? :)

-Oliver

[kfsone]

Wait ... Did the build fail because it recognized the correct error text printed by successful tests as errors??? :)

https://github.com/goccmack/gocc/runs/2017945678?check_suite_focus=true

[awalterschulze]

https://github.com/goccmack/gocc/runs/2017945678?check_suite_focus=true

No this just means the code was not regenerated, so there is a diff between the code you checked in and the code after running regeneration.

[awalterschulze]

Had a few moments to take Error.Error() for a quick spin and try to produce a quick mock-up:

https://gist.github.com/kfsone/ba8ce89ef2227c4230f1a9b40e3fe329

In my parser:

  if _, err = parser.Parse(lexer); err != nil {
      fmt.Println(fmt.Errorf("%s:%w", filename, err).Error())
  }

lets me produce some clear errors that are automatically linked to the source document and line in vim/emacs/vs/vsc/jetbrains and probably others.

Where there is one option for the token:

C:\Users\oliver\parsedef\baddata.blueprint:2:16: error: 8:17: error: expected typename; got: "hello"

or multiple:

C:\Users\oliver\parsedef\baddata.blueprint:2:16: error: expected one of: {, typename, true, false, float_literal, integer_literal, string_literal, or loc_literal; got: =

And a self-thrown error:

BlueprintName
    : typename
    | identifier << nil, fmt.Errorf("blueprint names must start with a capital letter; got: %s", ast.AttrToString($0)) >>
    ;

produces:

C:\Users\oliver\parsedef\baddata.blueprint:8:25: error: blueprint names must start with a capital letter; got: badnews

I'd like to quote literals, e.g. "{", "true", "false", but at the call to newError() there didn't appear to be any way to distinguish between a rule name and a literal value.

Is something like this interesting? Should it be gated behind an option? Would it be better isolating it as "don't write my error handlers for me"? Did I manage to avoid trampling every guideline you guys have? :)

-Oliver

There is some good ideas here. Will try to look at PRs and see if it is easier to comment on the concrete things.

[kfsone]

https://github.com/goccmack/gocc/runs/2017945678?check_suite_focus=true

No this just means the code was not regenerated, so there is a diff between the code you checked in and the code after running regeneration.

Doh! I'll add an || echo clause to the git diff so it's less well camouflaged :) I've also made a couple local changes to eliminate mutations during regenerate

[kfsone]

Sample of the additional error:

(base) oliver@Spud:/mnt/c/Users/oliver/go/src/github.com/kfsone/gocc$ echo -e "\n\n# wibble" >>Makefile
(base) oliver@Spud:/mnt/c/Users/oliver/go/src/github.com/kfsone/gocc$ make ci 2>&1 | tail -5
+
+
+# wibble
ERROR: commit and working copy differ after 'make regenerate'
make: *** [Makefile:37: ci] Error 22

[awalterschulze]

A bit unfortunate to lose the diff output, but on the other hand it has been confusing to many people before, so I think you can go ahead and add this in a separate pull request.

[kfsone]

The diff output is still there, i just limited it to the last 5 lines in the copy & paste (make | tail)

[awalterschulze]

Very nice :D