JSON Emitter: Use one indexed column numbers for edits

[MichaReiser]

I noticed in the byte-offsets refactor that the JsonEmitter uses one indexed column numbers for the diagnostic start and end locations but not for edits.

This PR changes the JsonEmitter to emit one-indexed column numbers for edits, as we already do for Message::location and Message::end_location.

Open questions

We'll need to change the LSP to subtract 1 from the columns in _parse_fix

https://github.com/charliermarsh/ruff-lsp/blob/6e44fadf8a5954adaf3c21af196fa195708ff912/ruff_lsp/server.py#L129-L150

@charliermarsh is there a way to get the ruff version in that method? If not, then I recommend adding a version that we increment whenever we make incompatible changes to the serialized message. We can then use it in the LSP to correctly compute the column offset.

I'll use the presence of the Fix::applicability field to detect if the Ruff version uses one or zero-based column indices.

See astral-sh/ruff-lsp#103

[MichaReiser]

Current dependencies on/for this PR:

• main
  • PR Add Fix::applicability to JSON output #4341
    • PR JSON Emitter: Use one indexed column numbers for edits #4007 👈

This comment was auto-generated by Graphite.

[github-actions]

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

[charliermarsh]

@MichaReiser - Yeah we do have a method to extract it from the executable (https://github.com/charliermarsh/ruff-lsp/blob/6e44fadf8a5954adaf3c21af196fa195708ff912/ruff_lsp/server.py#L903). I think what you're describing sounds preferable, though. I'd vote to go ahead and introduce that in the same release as this change.

(Relatedly, can we hold off on merging this until after the next release -- probably tomorrow?)

[MichaReiser]

(Relatedly, can we hold off on merging this until after the next release -- probably tomorrow?)

Sure

I think what you're describing sounds preferable, though. I'd vote to go ahead and introduce that in the same release as this change.

By this you mean introducing a version field? I reviewed the format of some other linters, and none seem to have a version field. There's also the complication that our top level node is an array, meaning we either:

• have to attach the version to every message which results in a lot of redundancy and, in some situation, makes it more difficult to check the version
• change the root to an object with, for now, two fields: results, and version.

Do you have a preference?

Note: I expect the JSON output to change significantly if we decide to rework our diagnostics. Having a version can then come in handy