Skip to content
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

Wrong locations in semantic highlighting when using preprocessors #1034

Closed
jboillot opened this issue Feb 21, 2023 · 1 comment
Closed

Wrong locations in semantic highlighting when using preprocessors #1034

jboillot opened this issue Feb 21, 2023 · 1 comment
Milestone

Comments

@jboillot
Copy link
Contributor

jboillot commented Feb 21, 2023

Hello!

I was using ocaml-lsp with cppo, a C-style preprocessor for OCaml while I noticed the semantic highlighting in files containing cppo code was broken:
Screenshot from 2023-02-21 15-41-18

After some investigation in the ocaml-lsp code I found this PR #932 that corrected the function lident

let lident ({ loc; _ } : Longident.t Loc.loc) rightmost_name
that parses long identifiers.
In order to remember spaces that may appear in long identifiers, it ignores the long identifier object given as a parameter (that already has spaces trimmed) and instead reads again the source M.source using the function source_excerpt
let source_excerpt ({ loc_start; loc_end; _ } : Loc.t) =
.

The problem is that, while using preprocessor directives, the location of the long identifier given as an argument is invalid (the location is specific to the code already preprocessed). Then the semantic highlighting returned is invalid.

I have tested to add to the lident function some logs after computing the source excerpt:

let lid = source_excerpt loc in
let lid2 = join (Longident.flatten lid2) in
debug (Printf.sprintf "I read \"%s\" while I could have read \"%s\"" lid lid2);

And here is an extract of the result:

I read "ec" while I could have read "()"
I read "e you" while I could have read "false"
I read "re" while I could have read "&&"
I read "warne" while I could have read "debug"
I read " i" while I could have read "()"

I am not sure what could be acceptable to do. Would it be possible to keep track of the source location? Or to keep the input text in the long identifier structure?

Thank you for reading!

PS: Maybe you have some clues @ulugbekna since you did the PR? :)

@jboillot
Copy link
Contributor Author

Fixed by #1049

@rgrinberg rgrinberg added this to the 1.16.0 milestone Mar 24, 2023
rgrinberg added a commit to rgrinberg/opam-repository that referenced this issue Jun 18, 2023
CHANGES:

## Fixes

- Disable code lens by default. The support can be re-enabled by explicitly
  setting it in the configuration. (ocaml/ocaml-lsp#1134)

- Fix initilization of `ocamlformat-rpc` in some edge cases when ocamlformat is
  initialized concurrently (ocaml/ocaml-lsp#1132)

- Kill unnecessary `$ dune ocaml-merlin` with SIGTERM rather than SIGKILL
  (ocaml/ocaml-lsp#1124)

- Refactor comment parsing to use `odoc-parser` and `cmarkit` instead of
  `octavius` and `omd` (ocaml/ocaml-lsp#1088)

  This allows users who migrated to omd 2.X to install ocaml-lsp-server in the
  same opam switch.

  We also slightly improved markdown generation support and fixed a couple in
  the generation of inline heading and module types.

- Allow opening documents that were already open. This is a workaround for
  neovim's lsp client (ocaml/ocaml-lsp#1067)

- Disable type annotation for functions (ocaml/ocaml-lsp#1054)

- Respect codeActionLiteralSupport capability (ocaml/ocaml-lsp#1046)

- Fix a document syncing issue when utf-16 is the position encoding (ocaml/ocaml-lsp#1004)

- Disable "Type-annotate" action for code that is already annotated.
  ([ocaml/ocaml-lsp#1037](ocaml/ocaml-lsp#1037)), fixes
  [ocaml/ocaml-lsp#1036](ocaml/ocaml-lsp#1036)

- Fix semantic highlighting of long identifiers when using preprocessors
  ([ocaml/ocaml-lsp#1049](ocaml/ocaml-lsp#1049), fixes
  [ocaml/ocaml-lsp#1034](ocaml/ocaml-lsp#1034))

- Fix the type of DocumentSelector in cram document registration (ocaml/ocaml-lsp#1068)

- Accept the `--clientProcessId` command line argument. (ocaml/ocaml-lsp#1074)

- Accept `--port` as a synonym for `--socket`. (ocaml/ocaml-lsp#1075)

- Fix connecting to dune rpc on Windows. (ocaml/ocaml-lsp#1080)

## Features

- Add "Remove type annotation" code action. (ocaml/ocaml-lsp#1039)

- Support settings through `didChangeConfiguration` notification (ocaml/ocaml-lsp#1103)

- Add "Extract local" and "Extract function" code actions. (ocaml/ocaml-lsp#870)

- Depend directly on `merlin-lib` 4.9 (ocaml/ocaml-lsp#1070)
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

No branches or pull requests

2 participants