-
Notifications
You must be signed in to change notification settings - Fork 234
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
refactor-open qualify uses short-paths #1313
Conversation
79dbf1f
to
2edb223
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Could you please rebase and add an entry to CHANGES.md? Then I'll merge.
@trefis Thanks for the review. I may have just found a bug with this implementation; let me test this slightly more, and I'll get back to you. |
2edb223
to
1cfa22d
Compare
c21a531
to
dc7c29d
Compare
dc7c29d
to
0cf7ebe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rewrite the changelog entry to not mention short paths, but to say something like this instead:
do not make paths absolute, simply prefix with the ident under the cursor
Otherwise this makes sense to me!
the same longident can appeart twice, so the second edit overwrites the first edit and this caused a bug, see the test for an example
0cf7ebe
to
c9d2302
Compare
refactor-open qualify uses short-paths
CHANGES: Mon Jul 26 11:13:37 AM CET 2021 + merlin binary - recover ill-typed patterns (ocaml/merlin#1317, ocaml/merlin#1342) - more accurate type-enclosing for methods (ocaml/merlin#1328, fixes ocaml/merlin#1124) - fix location of patterns in Occurrences (ocaml/merlin#1324, fixes ocaml/ocaml-lsp#375) - fix location of module definitions done via functors (ocaml/merlin#1329, fixes ocaml/merlin#1199) - fix -cmt-path dirs mistakenly added to build path (ocaml/merlin#1330) - add new module holes that can replace module expressions (ocaml/merlin#1333) - add a new command `construct` that builds a list of possible terms when called on a typed hole (ocaml/merlin#1318) - `refactor-open` improvements (ocaml/merlin#1313, ocaml/merlin#1314, ocaml/merlin#1366, ocaml/merlin#1372) - do not make paths absolute, simply prefix with the identifier under the cursor ```ocaml open Foo (* calling refactor-open qualify on this open *) let _ = Foo.bar (* previously could result in [Dune__exe.Foo.bar] *) ``` - do not return identical (duplicate) edits - do not return unnecessary edits that when applied do not change the document - handle record fields properly - handle multi-line paths - `unqualify` should not qualify - Handle `Persistent_env.Error` in `Typemod.initial_env` (ocaml/merlin#1355) - locate: reset global state from all entry points (ocaml/merlin#1364) - Windows: replace user name by its SID in socketnames (ocaml/merlin#1345, @ttamttam) + editor modes - vim: add a simple interface to the new `construct` command: `MerlinConstruct`. When several results are suggested, `<c-i>` and `<c-u>` to show more or less deep results. (ocaml/merlin#1318) - vim: add support for the `merlin-locate-type` command: `MerlinLocateType` (ocaml/merlin#1359) - emacs: add a simple interface to the new `construct` command: `merlin-construct`. (ocaml/merlin#1352) - emacs: add support for the `merlin-locate-type` command. (ocaml/merlin#1359) - emacs: fix issue with `merlin--highlight` and various minor improvements (ocaml/merlin#1367, @mattiase) + test suite - cover the new `construct` command (ocaml/merlin#1318)
CHANGES: Mon Jul 26 11:13:37 AM CET 2021 + merlin binary - recover ill-typed patterns (ocaml/merlin#1317, ocaml/merlin#1342) - more accurate type-enclosing for methods (ocaml/merlin#1328, fixes ocaml/merlin#1124) - fix location of patterns in Occurrences (ocaml/merlin#1324, fixes ocaml/ocaml-lsp#375) - fix location of module definitions done via functors (ocaml/merlin#1329, fixes ocaml/merlin#1199) - fix -cmt-path dirs mistakenly added to build path (ocaml/merlin#1330) - add new module holes that can replace module expressions (ocaml/merlin#1333) - add a new command `construct` that builds a list of possible terms when called on a typed hole (ocaml/merlin#1318) - `refactor-open` improvements (ocaml/merlin#1313, ocaml/merlin#1314, ocaml/merlin#1366, ocaml/merlin#1372) - do not make paths absolute, simply prefix with the identifier under the cursor ```ocaml open Foo (* calling refactor-open qualify on this open *) let _ = Foo.bar (* previously could result in [Dune__exe.Foo.bar] *) ``` - do not return identical (duplicate) edits - do not return unnecessary edits that when applied do not change the document - handle record fields properly - handle multi-line paths - `unqualify` should not qualify - Handle `Persistent_env.Error` in `Typemod.initial_env` (ocaml/merlin#1355) - locate: reset global state from all entry points (ocaml/merlin#1364) - Windows: replace user name by its SID in socketnames (ocaml/merlin#1345, @ttamttam) + editor modes - vim: add a simple interface to the new `construct` command: `MerlinConstruct`. When several results are suggested, `<c-i>` and `<c-u>` to show more or less deep results. (ocaml/merlin#1318) - vim: add support for the `merlin-locate-type` command: `MerlinLocateType` (ocaml/merlin#1359) - emacs: add a simple interface to the new `construct` command: `merlin-construct`. (ocaml/merlin#1352) - emacs: add support for the `merlin-locate-type` command. (ocaml/merlin#1359) - emacs: fix issue with `merlin--highlight` and various minor improvements (ocaml/merlin#1367, @mattiase) + test suite - cover the new `construct` command (ocaml/merlin#1318)
CHANGES: Mon Jul 26 11:13:37 AM CET 2021 + merlin binary - recover ill-typed patterns (ocaml/merlin#1317, ocaml/merlin#1342) - more accurate type-enclosing for methods (ocaml/merlin#1328, fixes ocaml/merlin#1124) - fix location of patterns in Occurrences (ocaml/merlin#1324, fixes ocaml/ocaml-lsp#375) - fix location of module definitions done via functors (ocaml/merlin#1329, fixes ocaml/merlin#1199) - fix -cmt-path dirs mistakenly added to build path (ocaml/merlin#1330) - add new module holes that can replace module expressions (ocaml/merlin#1333) - add a new command `construct` that builds a list of possible terms when called on a typed hole (ocaml/merlin#1318) - `refactor-open` improvements (ocaml/merlin#1313, ocaml/merlin#1314, ocaml/merlin#1366, ocaml/merlin#1372) - do not make paths absolute, simply prefix with the identifier under the cursor ```ocaml open Foo (* calling refactor-open qualify on this open *) let _ = Foo.bar (* previously could result in [Dune__exe.Foo.bar] *) ``` - do not return identical (duplicate) edits - do not return unnecessary edits that when applied do not change the document - handle record fields properly - handle multi-line paths - `unqualify` should not qualify - Handle `Persistent_env.Error` in `Typemod.initial_env` (ocaml/merlin#1355) - locate: reset global state from all entry points (ocaml/merlin#1364) - Windows: replace user name by its SID in socketnames (ocaml/merlin#1345, @ttamttam) + editor modes - vim: add a simple interface to the new `construct` command: `MerlinConstruct`. When several results are suggested, `<c-i>` and `<c-u>` to show more or less deep results. (ocaml/merlin#1318) - vim: add support for the `merlin-locate-type` command: `MerlinLocateType` (ocaml/merlin#1359) - emacs: add a simple interface to the new `construct` command: `merlin-construct`. (ocaml/merlin#1352) - emacs: add support for the `merlin-locate-type` command. (ocaml/merlin#1359) - emacs: fix issue with `merlin--highlight` and various minor improvements (ocaml/merlin#1367, @mattiase) + test suite - cover the new `construct` command (ocaml/merlin#1318)
CHANGES: Mon Jul 26 11:13:37 AM CET 2021 + merlin binary - recover ill-typed patterns (ocaml/merlin#1317, ocaml/merlin#1342) - more accurate type-enclosing for methods (ocaml/merlin#1328, fixes ocaml/merlin#1124) - fix location of patterns in Occurrences (ocaml/merlin#1324, fixes ocaml/ocaml-lsp#375) - fix location of module definitions done via functors (ocaml/merlin#1329, fixes ocaml/merlin#1199) - fix -cmt-path dirs mistakenly added to build path (ocaml/merlin#1330) - add new module holes that can replace module expressions (ocaml/merlin#1333) - add a new command `construct` that builds a list of possible terms when called on a typed hole (ocaml/merlin#1318) - `refactor-open` improvements (ocaml/merlin#1313, ocaml/merlin#1314, ocaml/merlin#1366, ocaml/merlin#1372) - do not make paths absolute, simply prefix with the identifier under the cursor ```ocaml open Foo (* calling refactor-open qualify on this open *) let _ = Foo.bar (* previously could result in [Dune__exe.Foo.bar] *) ``` - do not return identical (duplicate) edits - do not return unnecessary edits that when applied do not change the document - handle record fields properly - handle multi-line paths - `unqualify` should not qualify - Handle `Persistent_env.Error` in `Typemod.initial_env` (ocaml/merlin#1355) - locate: reset global state from all entry points (ocaml/merlin#1364) - Windows: replace user name by its SID in socketnames (ocaml/merlin#1345, @ttamttam) + editor modes - vim: add a simple interface to the new `construct` command: `MerlinConstruct`. When several results are suggested, `<c-i>` and `<c-u>` to show more or less deep results. (ocaml/merlin#1318) - vim: add support for the `merlin-locate-type` command: `MerlinLocateType` (ocaml/merlin#1359) - emacs: add a simple interface to the new `construct` command: `merlin-construct`. (ocaml/merlin#1352) - emacs: add support for the `merlin-locate-type` command. (ocaml/merlin#1359) - emacs: fix issue with `merlin--highlight` and various minor improvements (ocaml/merlin#1367, @mattiase) + test suite - cover the new `construct` command (ocaml/merlin#1318)
CHANGES: Mon Jul 26 11:13:37 AM CET 2021 + merlin binary - recover ill-typed patterns (ocaml/merlin#1317, ocaml/merlin#1342) - more accurate type-enclosing for methods (ocaml/merlin#1328, fixes ocaml/merlin#1124) - fix location of patterns in Occurrences (ocaml/merlin#1324, fixes ocaml/ocaml-lsp#375) - fix location of module definitions done via functors (ocaml/merlin#1329, fixes ocaml/merlin#1199) - fix -cmt-path dirs mistakenly added to build path (ocaml/merlin#1330) - add new module holes that can replace module expressions (ocaml/merlin#1333) - add a new command `construct` that builds a list of possible terms when called on a typed hole (ocaml/merlin#1318) - `refactor-open` improvements (ocaml/merlin#1313, ocaml/merlin#1314, ocaml/merlin#1366, ocaml/merlin#1372) - do not make paths absolute, simply prefix with the identifier under the cursor ```ocaml open Foo (* calling refactor-open qualify on this open *) let _ = Foo.bar (* previously could result in [Dune__exe.Foo.bar] *) ``` - do not return identical (duplicate) edits - do not return unnecessary edits that when applied do not change the document - handle record fields properly - handle multi-line paths - `unqualify` should not qualify - Handle `Persistent_env.Error` in `Typemod.initial_env` (ocaml/merlin#1355) - locate: reset global state from all entry points (ocaml/merlin#1364) - Windows: replace user name by its SID in socketnames (ocaml/merlin#1345, @ttamttam) + editor modes - vim: add a simple interface to the new `construct` command: `MerlinConstruct`. When several results are suggested, `<c-i>` and `<c-u>` to show more or less deep results. (ocaml/merlin#1318) - vim: add support for the `merlin-locate-type` command: `MerlinLocateType` (ocaml/merlin#1359) - emacs: add a simple interface to the new `construct` command: `merlin-construct`. (ocaml/merlin#1352) - emacs: add support for the `merlin-locate-type` command. (ocaml/merlin#1359) - emacs: fix issue with `merlin--highlight` and various minor improvements (ocaml/merlin#1367, @mattiase) + test suite - cover the new `construct` command (ocaml/merlin#1318)
CHANGES: Mon Jul 26 11:13:37 AM CET 2021 + merlin binary - recover ill-typed patterns (ocaml/merlin#1317, ocaml/merlin#1342) - more accurate type-enclosing for methods (ocaml/merlin#1328, fixes ocaml/merlin#1124) - fix location of patterns in Occurrences (ocaml/merlin#1324, fixes ocaml/ocaml-lsp#375) - fix location of module definitions done via functors (ocaml/merlin#1329, fixes ocaml/merlin#1199) - fix -cmt-path dirs mistakenly added to build path (ocaml/merlin#1330) - add new module holes that can replace module expressions (ocaml/merlin#1333) - add a new command `construct` that builds a list of possible terms when called on a typed hole (ocaml/merlin#1318) - `refactor-open` improvements (ocaml/merlin#1313, ocaml/merlin#1314, ocaml/merlin#1366, ocaml/merlin#1372) - do not make paths absolute, simply prefix with the identifier under the cursor ```ocaml open Foo (* calling refactor-open qualify on this open *) let _ = Foo.bar (* previously could result in [Dune__exe.Foo.bar] *) ``` - do not return identical (duplicate) edits - do not return unnecessary edits that when applied do not change the document - handle record fields properly - handle multi-line paths - `unqualify` should not qualify - Handle `Persistent_env.Error` in `Typemod.initial_env` (ocaml/merlin#1355) - locate: reset global state from all entry points (ocaml/merlin#1364) - Windows: replace user name by its SID in socketnames (ocaml/merlin#1345, @ttamttam) + editor modes - vim: add a simple interface to the new `construct` command: `MerlinConstruct`. When several results are suggested, `<c-i>` and `<c-u>` to show more or less deep results. (ocaml/merlin#1318) - vim: add support for the `merlin-locate-type` command: `MerlinLocateType` (ocaml/merlin#1359) - emacs: add a simple interface to the new `construct` command: `merlin-construct`. (ocaml/merlin#1352) - emacs: add support for the `merlin-locate-type` command. (ocaml/merlin#1359) - emacs: fix issue with `merlin--highlight` and various minor improvements (ocaml/merlin#1367, @mattiase) + test suite - cover the new `construct` command (ocaml/merlin#1318)
CHANGES: Mon Jul 26 04:45:37 PM CET 2021 + merlin binary - recover ill-typed patterns (ocaml/merlin#1317, ocaml/merlin#1342) - more accurate type-enclosing for methods (ocaml/merlin#1328, fixes ocaml/merlin#1124) - fix location of patterns in Occurrences (ocaml/merlin#1324, fixes ocaml/ocaml-lsp#375) - fix location of module definitions done via functors (ocaml/merlin#1329, fixes ocaml/merlin#1199) - fix -cmt-path dirs mistakenly added to build path (ocaml/merlin#1330) - add new module holes that can replace module expressions (ocaml/merlin#1333) - add a new command `construct` that builds a list of possible terms when called on a typed hole (ocaml/merlin#1318) - `refactor-open` improvements (ocaml/merlin#1313, ocaml/merlin#1314, ocaml/merlin#1366, ocaml/merlin#1372) - do not make paths absolute, simply prefix with the identifier under the cursor ```ocaml open Foo (* calling refactor-open qualify on this open *) let _ = Foo.bar (* previously could result in [Dune__exe.Foo.bar] *) ``` - do not return identical (duplicate) edits - do not return unnecessary edits that when applied do not change the document - handle record fields properly - handle multi-line paths - `unqualify` should not qualify - Handle `Persistent_env.Error` in `Typemod.initial_env` (ocaml/merlin#1355) - locate: reset global state from all entry points (ocaml/merlin#1364) - Windows: replace user name by its SID in socketnames (ocaml/merlin#1345, @ttamttam) + editor modes - vim: add a simple interface to the new `construct` command: `MerlinConstruct`. When several results are suggested, `<c-i>` and `<c-u>` can be use to change the depth of the recursive construction. (ocaml/merlin#1318) - vim: add support for the `merlin-locate-type` command: `MerlinLocateType` (ocaml/merlin#1359) - emacs: add a simple interface to the new `construct` command: `merlin-construct`. (ocaml/merlin#1352) - emacs: add support for the `merlin-locate-type` command. (ocaml/merlin#1359) - emacs: fix issue with `merlin--highlight` and various minor improvements (ocaml/merlin#1367, @mattiase) + test suite - cover the new `construct` command (ocaml/merlin#1318) - disable tests failing in Opam's CI due to nested dune projects (ocaml/merlin#1373)
CHANGES: Mon Jul 26 04:45:37 PM CET 2021 + merlin binary - recover ill-typed patterns (ocaml/merlin#1317, ocaml/merlin#1342) - more accurate type-enclosing for methods (ocaml/merlin#1328, fixes ocaml/merlin#1124) - fix location of patterns in Occurrences (ocaml/merlin#1324, fixes ocaml/ocaml-lsp#375) - fix location of module definitions done via functors (ocaml/merlin#1329, fixes ocaml/merlin#1199) - fix -cmt-path dirs mistakenly added to build path (ocaml/merlin#1330) - add new module holes that can replace module expressions (ocaml/merlin#1333) - add a new command `construct` that builds a list of possible terms when called on a typed hole (ocaml/merlin#1318) - `refactor-open` improvements (ocaml/merlin#1313, ocaml/merlin#1314, ocaml/merlin#1366, ocaml/merlin#1372) - do not make paths absolute, simply prefix with the identifier under the cursor ```ocaml open Foo (* calling refactor-open qualify on this open *) let _ = Foo.bar (* previously could result in [Dune__exe.Foo.bar] *) ``` - do not return identical (duplicate) edits - do not return unnecessary edits that when applied do not change the document - handle record fields properly - handle multi-line paths - `unqualify` should not qualify - Handle `Persistent_env.Error` in `Typemod.initial_env` (ocaml/merlin#1355) - locate: reset global state from all entry points (ocaml/merlin#1364) - Windows: replace user name by its SID in socketnames (ocaml/merlin#1345, @ttamttam) + editor modes - vim: add a simple interface to the new `construct` command: `MerlinConstruct`. When several results are suggested, `<c-i>` and `<c-u>` can be use to change the depth of the recursive construction. (ocaml/merlin#1318) - vim: add support for the `merlin-locate-type` command: `MerlinLocateType` (ocaml/merlin#1359) - emacs: add a simple interface to the new `construct` command: `merlin-construct`. (ocaml/merlin#1352) - emacs: add support for the `merlin-locate-type` command. (ocaml/merlin#1359) - emacs: fix issue with `merlin--highlight` and various minor improvements (ocaml/merlin#1367, @mattiase) + test suite - cover the new `construct` command (ocaml/merlin#1318) - disable tests failing in Opam's CI due to nested dune projects (ocaml/merlin#1373)
CHANGES: Mon Jul 26 04:45:37 PM CET 2021 + merlin binary - recover ill-typed patterns (ocaml/merlin#1317, ocaml/merlin#1342) - more accurate type-enclosing for methods (ocaml/merlin#1328, fixes ocaml/merlin#1124) - fix location of patterns in Occurrences (ocaml/merlin#1324, fixes ocaml/ocaml-lsp#375) - fix location of module definitions done via functors (ocaml/merlin#1329, fixes ocaml/merlin#1199) - fix -cmt-path dirs mistakenly added to build path (ocaml/merlin#1330) - add new module holes that can replace module expressions (ocaml/merlin#1333) - add a new command `construct` that builds a list of possible terms when called on a typed hole (ocaml/merlin#1318) - `refactor-open` improvements (ocaml/merlin#1313, ocaml/merlin#1314, ocaml/merlin#1366, ocaml/merlin#1372) - do not make paths absolute, simply prefix with the identifier under the cursor ```ocaml open Foo (* calling refactor-open qualify on this open *) let _ = Foo.bar (* previously could result in [Dune__exe.Foo.bar] *) ``` - do not return identical (duplicate) edits - do not return unnecessary edits that when applied do not change the document - handle record fields properly - handle multi-line paths - `unqualify` should not qualify - Handle `Persistent_env.Error` in `Typemod.initial_env` (ocaml/merlin#1355) - locate: reset global state from all entry points (ocaml/merlin#1364) - Windows: replace user name by its SID in socketnames (ocaml/merlin#1345, @ttamttam) + editor modes - vim: add a simple interface to the new `construct` command: `MerlinConstruct`. When several results are suggested, `<c-i>` and `<c-u>` can be use to change the depth of the recursive construction. (ocaml/merlin#1318) - vim: add support for the `merlin-locate-type` command: `MerlinLocateType` (ocaml/merlin#1359) - emacs: add a simple interface to the new `construct` command: `merlin-construct`. (ocaml/merlin#1352) - emacs: add support for the `merlin-locate-type` command. (ocaml/merlin#1359) - emacs: fix issue with `merlin--highlight` and various minor improvements (ocaml/merlin#1367, @mattiase) + test suite - cover the new `construct` command (ocaml/merlin#1318) - disable tests failing in Opam's CI due to nested dune projects (ocaml/merlin#1373)
refactor-open qualify
uses a "full path" to qualify modules, e.g.,I think this is not what a user wants when they use this command.
This PR makes the command to qualify paths up to the leftmost ident in the open expression, e.g., up to
Import
in the above example despite the full path beingDune__exe.Import.Option.value_exn
.Notes: