Permalink
Browse files

No longer error when formatting invalid syntax

Summary:
If LSP issues a formatting request at a given line+char, but we can't calculate a better format for the file at this time due to syntax errors in it, what should we do?

Previously we returned an error response to the request because the code wasn't syntactically valid (which happens an awful lot of the time while you're typing!)

I reckon we should instead return "success and there are no edits to apply". That feels more in keeping with the LSP attitude of not caring at all about the meaning of the language. It's also nice because it means our logs will no longer be dominated by failed-to-parse errors.

(Syntax errors are properly reported to the user via publishDiagnostics. I don't think they should also be reported to the formatting routines within VSCode/Atom).

Reviewed By: arxanas

Differential Revision: D7686887

fbshipit-source-id: e7f2bb72d0777e85dab0e10ed9998366fe086470
  • Loading branch information...
ljw1004 authored and hhvm-bot committed Apr 21, 2018
1 parent 4b981a6 commit 8b6cb6c1af3c0441cab1ee910c1859e22382fdbd
Showing with 10 additions and 0 deletions.
  1. +10 −0 hphp/hack/src/client/clientLsp.ml
@@ -1131,6 +1131,16 @@ let do_formatting_common
rpc conn ref_unblocked_time command
in
match response with
| Error "File failed to parse without errors" ->
(* If LSP issues a formatting request at a given line+char, but we can't *)
(* calculate a better format for the file due to syntax errors in it, *)
(* then we should return "success and there are no edits to apply" *)
(* rather than "error". *)
(* TODO: let's eliminate hh_format, and incorporate hackfmt into the *)
(* hh_client binary itself, and make make "hackfmt" just a wrapper for *)
(* "hh_client format", and then make it return proper error that we can *)
(* pattern-match upon, rather than hard-coding the string... *)
[]
| Error message ->
raise (Error.InternalError message)
| Ok r ->

0 comments on commit 8b6cb6c

Please sign in to comment.