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

Print argument parsing errors #1073

Merged
merged 1 commit into from
Dec 20, 2019

Conversation

rgrinberg
Copy link
Member

This should save people time debugging. It also caught some silent errors
in some tests. The commit history describes includes the details

@trefis
Copy link
Contributor

trefis commented Dec 20, 2019

I don't think that's the right way to do things. Printing on stderr means that garbage will appear over your buffer if you're using vim (I'm not sure what happens in emacs)

We have a dedicated "notification" (or something like that) field in the json message we print on stdout as a result, failures should go in there.

@trefis
Copy link
Contributor

trefis commented Dec 20, 2019

We have a dedicated "notification" (or something like that) field in the json message we print on stdout as a result, failures should go in there.

Btw, I'm actually working on that particular area at the moment (though I have nothing to show for it yet), with the aim to also report errors when reading the configuration for .merlin files or when trying to reach the build system.
So I think you could put this work on hold for the next couple of weeks, I hope to push something in the new year.

@rgrinberg
Copy link
Member Author

rgrinberg commented Dec 20, 2019 via email

@trefis
Copy link
Contributor

trefis commented Dec 20, 2019

Yes, that sounds good :)

* -look-for <ml|mli>
* -target <next|pev>

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
@rgrinberg
Copy link
Member Author

rgrinberg commented Dec 20, 2019

The PR now contains only test fixes

@trefis trefis merged commit a451081 into ocaml:master Dec 20, 2019
@let-def
Copy link
Contributor

let-def commented Apr 14, 2020

Actually, printing on stderr is now fine with both vim and emacs.
In vim it is redirected to an hidden logging buffer (displayed with :b *merlin-log* or :MerlinDebugEnable).
In Emacs it is dropped unless logging is enabled, in which case it is redirected to the logging buffer.

In the future, it is fine to use stderr for debug/log/notifications.

let-def added a commit to let-def/opam-repository that referenced this pull request Apr 15, 2020
CHANGES:

Tue Apr 14 15:25:05 CEST 2020

  + backend
    - full support from OCaml 4.02 to OCaml 4.10 (ocaml/merlin#1117, ocaml/merlin#1127)
    - fix desynchronized cache (ocaml/merlin#1120)
    - short path for OCaml 4.09 and OCaml 4.10 (ocaml/merlin#1082, ocaml/merlin#1117)
    - catch and test environment initialization errors (ocaml/merlin#1083, ocaml/merlin#1130)
    - restore type levels after recovery (ocaml/merlin#1092)
  + frontend
    - fix syntax errors in 4.08 and 4.09 (ocaml/merlin#1081)
    - complete-prefix command accepts -kind option to filter results (ocaml/merlin#1071)
    - code cleanup (ocaml/merlin#1093, ocaml/merlin#1079, ocaml/merlin#1112)
    - better handling of expression and pattern extra nodes during browse tree
      traversal (ocaml/merlin#1091, ocaml/merlin#1121)
    - improve context detection (e.g. appropriate namespace for lookup) for
      various queries (ocaml/merlin#1104, ocaml/merlin#1110)
    - add stdlib to locate source path (ocaml/merlin#1085)
  + editor modes
    - vim: tweak heuristic to select python version (ocaml/merlin#1111)
    - emacs: marlin/call
    - lsp: move server to its own repository (ocaml/merlin#1069),
      https://github.com/ocaml/ocaml-lsp
  + testsuite
    - dune rules for the testsuite are now generated, deterministic and
      can be run individually (ocaml/merlin#1068, ocaml/merlin#1070, ocaml/merlin#1072)
    - fix incorrect command-line arguments in tests (ocaml/merlin#1073)
    - better coverage of frontend features (ocaml/merlin#1075, ocaml/merlin#1078, ocaml/merlin#1088, ocaml/merlin#1089, ocaml/merlin#1126)

Build no longer relies on implicit transitive_deps (ocaml/merlin#1065).
let-def added a commit to let-def/opam-repository that referenced this pull request Apr 15, 2020
CHANGES:

Tue Apr 14 15:25:05 CEST 2020

  + ocaml support
    - full support from OCaml 4.02 to OCaml 4.10 (ocaml/merlin#1117, ocaml/merlin#1127)
    - fix desynchronized cache (ocaml/merlin#1120)
    - short path for OCaml 4.09 and OCaml 4.10 (ocaml/merlin#1082, ocaml/merlin#1117)
    - catch and test environment initialization errors (ocaml/merlin#1083, ocaml/merlin#1130)
    - restore type levels after recovery (ocaml/merlin#1092)
  + merlin binary
    - fix syntax errors in 4.08 and 4.09 (ocaml/merlin#1081)
    - complete-prefix command accepts -kind option to filter results (ocaml/merlin#1071)
    - code cleanup (ocaml/merlin#1093, ocaml/merlin#1079, ocaml/merlin#1112)
    - better handling of expression and pattern extra nodes during browse tree
      traversal (ocaml/merlin#1091, ocaml/merlin#1121)
    - improve context detection (e.g. appropriate namespace for lookup) for
      various queries (ocaml/merlin#1104, ocaml/merlin#1110)
    - add stdlib to locate source path (ocaml/merlin#1085)
  + editor modes
    - vim: tweak heuristic to select python version (ocaml/merlin#1111)
    - emacs: marlin/call
    - lsp: move server to its own repository (ocaml/merlin#1069),
      https://github.com/ocaml/ocaml-lsp
  + test suite
    - dune rules for the test suite are now generated, deterministic and
      can be run individually (ocaml/merlin#1068, ocaml/merlin#1070, ocaml/merlin#1072)
    - fix incorrect command-line arguments in tests (ocaml/merlin#1073)
    - better coverage of frontend features (ocaml/merlin#1075, ocaml/merlin#1078, ocaml/merlin#1088, ocaml/merlin#1089, ocaml/merlin#1126)

Build no longer relies on implicit transitive_deps (ocaml/merlin#1065).
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.

3 participants