Skip to content

Add Grace_json_renderer - #84

Merged
johnyob merged 1 commit into
johnyob:mainfrom
WardBrian:issue/82-json-renderer
Mar 23, 2026
Merged

Add Grace_json_renderer#84
johnyob merged 1 commit into
johnyob:mainfrom
WardBrian:issue/82-json-renderer

Conversation

@WardBrian

@WardBrian WardBrian commented Mar 11, 2026

Copy link
Copy Markdown
Contributor

Closes #82.

For testing, I am re-using the ansi renderer tests to a large degree. Obviously they could be simplified, since the edge cases are much more limited in this renderer.

Example:

$ dune exec -- ./examples/main.exe --json
{
  "error_code": "E001",
  "severity": "error",
  "message": "`match` cases have incompatible types",
  "notes": [],
  "labels": [
    {
      "range": {
        "source": "fizz.ml",
        "start": { "line": 7, "column": 14 },
        "end": { "line": 8, "column": 1 }
      },
      "priority": "primary",
      "message": "expected `[> `Buzz | `Fizz | `Fizz_buzz]`, found `int`"
    },
    {
      "range": {
        "source": "fizz.ml",
        "start": { "line": 3, "column": 4 },
        "end": { "line": 8, "column": 1 }
      },
      "priority": "secondary",
      "message": "`match` cases have incompatible types"
    },
    {
      "range": {
        "source": "fizz.ml",
        "start": { "line": 4, "column": 14 },
        "end": { "line": 5, "column": 1 }
      },
      "priority": "secondary",
      "message": "this is found to be of type `[> `Fizz_buzz]`"
    },
    {
      "range": {
        "source": "fizz.ml",
        "start": { "line": 5, "column": 14 },
        "end": { "line": 6, "column": 1 }
      },
      "priority": "secondary",
      "message": "this is found to be of type `[> `Fizz]`"
    },
    {
      "range": {
        "source": "fizz.ml",
        "start": { "line": 6, "column": 14 },
        "end": { "line": 7, "column": 1 }
      },
      "priority": "secondary",
      "message": "this is found to be of type `[> `Buzz]`"
    }
  ]
}

@WardBrian WardBrian mentioned this pull request Mar 13, 2026

@johnyob johnyob left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! 🙏

Just a few comments (mostly style / nits), the logic all looks correct 🚀

Comment thread lib/json_renderer/grace_json_renderer.mli Outdated
Comment thread lib/json_renderer/snippet.ml Outdated
Comment thread lib/json_renderer/snippet.ml Outdated
Comment thread lib/json_renderer/snippet.ml Outdated
Comment thread lib/json_renderer/snippet.ml Outdated
Comment thread lib/source_reader/grace_source_reader.mli Outdated

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to avoid duplicating test data (we should probably start putting these code snippets in dedicated files anyway), but that can be done in a separate PR :)

Comment thread lib/json_renderer/grace_json_renderer.mli Outdated
@WardBrian

Copy link
Copy Markdown
Contributor Author

@johnyob thanks for the comments. I took all your suggestions minus the deduplication of tests for now.

@johnyob johnyob left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm 🚀

Seems like you need to make CI happy (likely your commit message format). After that we can merge :)

@WardBrian
WardBrian force-pushed the issue/82-json-renderer branch from 255a377 to b5542ff Compare March 23, 2026 13:48
@WardBrian

Copy link
Copy Markdown
Contributor Author

Thanks @johnyob -- I squashed everything into one feat commit which should hopefully please the CI

@johnyob
johnyob merged commit fe7018b into johnyob:main Mar 23, 2026
@WardBrian
WardBrian deleted the issue/82-json-renderer branch March 23, 2026 14:35
@jonsterling

jonsterling commented Apr 21, 2026

Copy link
Copy Markdown

One thing I would like to raise is that AFAIK major language clients, such as that of Visual Studio Code, only properly support UTF-16 offsets. So integrating Grace with the language server protocol will in practice require switching to UTF-16 offsets, because VS Code is the de facto 'canonical' instance of LSP.

Does it seem like there is a chance of making this change, or of at least providing this option? /cc @kentookura.

johnyob added a commit to johnyob/opam-repository that referenced this pull request Jul 23, 2026
CHANGES:

- feat(renderer): add boxing to trailing labels ([johnyob/grace#92](johnyob/grace#92))
- fix(renderer): print unicode and ansi-styled snippets as correct lengths ([johnyob/grace#88](johnyob/grace#88))
- feat(renderer): add boxing to multi-line labels ([johnyob/grace#88](johnyob/grace#88))
- feat(json_conv): use `Yojson.Basic.t` ([johnyob/grace#87](johnyob/grace#87))
- feat(json_conv): add `Grace_json_conv` for conversion to Yojson ([johnyob/grace#84](johnyob/grace#84))
- fix(core): dont catch `Sys_error` in `Source.length` if the fail doesn't exist ([johnyob/grace#81](johnyob/grace#81))
- feat(renderer): support configurable contextual lines ([johnyob/grace#74](johnyob/grace#74))
johnyob added a commit to johnyob/opam-repository that referenced this pull request Jul 24, 2026
CHANGES:

- feat(renderer): add boxing to trailing labels ([johnyob/grace#92](johnyob/grace#92))
- fix(renderer): print unicode and ansi-styled snippets as correct lengths ([johnyob/grace#88](johnyob/grace#88))
- feat(renderer): add boxing to multi-line labels ([johnyob/grace#88](johnyob/grace#88))
- feat(json_conv): use `Yojson.Basic.t` ([johnyob/grace#87](johnyob/grace#87))
- feat(json_conv): add `Grace_json_conv` for conversion to Yojson ([johnyob/grace#84](johnyob/grace#84))
- fix(core): dont catch `Sys_error` in `Source.length` if the fail doesn't exist ([johnyob/grace#81](johnyob/grace#81))
- feat(renderer): support configurable contextual lines ([johnyob/grace#74](johnyob/grace#74))
johnyob added a commit to johnyob/opam-repository that referenced this pull request Jul 27, 2026
CHANGES:

- feat(renderer): add boxing to trailing labels ([johnyob/grace#92](johnyob/grace#92))
- fix(renderer): print unicode and ansi-styled snippets as correct lengths ([johnyob/grace#88](johnyob/grace#88))
- feat(renderer): add boxing to multi-line labels ([johnyob/grace#88](johnyob/grace#88))
- feat(json_conv): use `Yojson.Basic.t` ([johnyob/grace#87](johnyob/grace#87))
- feat(json_conv): add `Grace_json_conv` for conversion to Yojson ([johnyob/grace#84](johnyob/grace#84))
- fix(core): dont catch `Sys_error` in `Source.length` if the fail doesn't exist ([johnyob/grace#81](johnyob/grace#81))
- feat(renderer): support configurable contextual lines ([johnyob/grace#74](johnyob/grace#74))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JSON renderer

3 participants