Skip to content

Commit 0f26849

Browse files
committed
Fix IDE formatting of correctly formatted files with trailing newlines
Since D60978552, we special-case removing a range of lines during IDE formatting so that T188437747 does not rear its ugly head. However, when document-formatting a file that is already correctly formatted save for some spurious trailing newlines, this logic ends up consuming the last non-newline line. So, only shift the start offset if the last changed line wasn't a trailing line that got removed. Add a test and wire up `hh_single_ide_format` with the OSS dune build so that IDE format tests can be run.
1 parent 4b934e0 commit 0f26849

File tree

4 files changed

+42
-1
lines changed

4 files changed

+42
-1
lines changed

hphp/hack/Makefile.dune

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ build-hack$(2):
3535
dune build \
3636
src/hh_server.$(1) \
3737
src/hh_client.$(1) \
38+
src/hh_single_ide_format.$(1) \
3839
src/hh_single_type_check.$(1) \
3940
src/hackfmt.$(1) \
4041
src/hh_parse.$(1) \
@@ -44,6 +45,7 @@ copy-hack$(2)-files: build-hack$(2)
4445
mkdir -p "$(HACK_BIN_DIR)"
4546
${COPY_EXE} "$(DUNE_BUILD_DIR)/default/hack/src/hh_server.$(1)" "$(HACK_BIN_DIR)/hh_server$(ext)"
4647
${COPY_EXE} "$(DUNE_BUILD_DIR)/default/hack/src/hh_client.$(1)" "$(HACK_BIN_DIR)/hh_client$(ext)"
48+
${COPY_EXE} "$(DUNE_BUILD_DIR)/default/hack/src/hh_single_ide_format.$(1)" "$(HACK_BIN_DIR)/hh_single_ide_format$(ext)"
4749
${COPY_EXE} "$(DUNE_BUILD_DIR)/default/hack/src/hh_single_type_check.$(1)" "$(HACK_BIN_DIR)/hh_single_type_check$(ext)"
4850
${COPY_EXE} "$(DUNE_BUILD_DIR)/default/hack/src/hackfmt.$(1)" "$(HACK_BIN_DIR)/hackfmt$(ext)"
4951
${COPY_EXE} "$(DUNE_BUILD_DIR)/default/hack/src/hh_parse.$(1)" "$(HACK_BIN_DIR)/hh_parse$(ext)"

hphp/hack/src/client/ide_service/ide_format.ml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ let minimal_edit (old_src : string) (new_src : string) :
179179
(List.length old_src_lines - range_end_line)
180180
in
181181
let (new_text, start_line_adjustment) =
182-
if List.is_empty new_src_novel_lines then
182+
if List.is_empty new_src_novel_lines && range_end_line < List.length new_src_lines then
183183
(*
184184
Empty `new_src_novel_lines` does not mean that there was no change
185185
(otherwise we wouldn't have reached here),
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?hh
2+
// Formatting an otherwise-correctly formatted file with trailing newlines
3+
// should not devour the last line before the newlines.
4+
class A {
5+
public function baz(): int {
6+
return 5;
7+
}
8+
}
9+
10+
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
> lines and columns in ranges are 1-indexed
2+
received code:
3+
1 <?hh
4+
2 // Formatting an otherwise-correctly formatted file with trailing newlines
5+
3 // should not devour the last line before the newlines.
6+
4 class A {
7+
5 public function baz(): int {
8+
6 return 5;
9+
7 }
10+
8 }
11+
9
12+
10
13+
14+
New text at range 9:1-11:1 (1-indexed) is
15+
'
16+
'
17+
18+
Code after applying language server edits:
19+
'<?hh
20+
// Formatting an otherwise-correctly formatted file with trailing newlines
21+
// should not devour the last line before the newlines.
22+
class A {
23+
public function baz(): int {
24+
return 5;
25+
}
26+
}
27+
'
28+
29+
Matched Hackfmt result

0 commit comments

Comments
 (0)