Skip to content

Commit fd9b133

Browse files
mheiberfacebook-github-bot
authored andcommitted
strip non-hack marker when converting back to notebook
Summary: ## What Fix bug [reported by a user](https://fb.workplace.com/groups/768246428434224/posts/1082807136978150/?comment_id=1083743046884559&reply_comment_id=1084402066818657): When converting notebook->hack we were wrapping python cells in comments, then forgetting to strip those comments when converting hack->notebook. ## implementation: - We don't want to strip comments from code cells that the user put there, so we use a special `/*non_hack:` comment to distinguish. - We *could* parse cell metadata to look for the cell language, but we try to limit our dependence on metadata and this way is much simpler Python cells now look like this when converted to Hack: ``` //bento-cell:{"cell_bento_metadata":{"language":"python"},"cell_type":"code","id":4} /*non_hack: %%python print('Hello from python') */ //bento-cell-end ``` Reviewed By: nt591 Differential Revision: D73524434 fbshipit-source-id: cf54fdab111038039bd12711c31ba6682641231a
1 parent f406258 commit fd9b133

13 files changed

+53
-14
lines changed

hphp/hack/src/notebook_convert/internal/notebook_chunk.ml

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,26 @@ let cell_end_regexp = Str.regexp ({|^ *|} ^ bento_cell_end_comment)
5050

5151
let is_chunk_end_comment s = Str.string_match cell_end_regexp s 0
5252

53-
let wrap_in_comment = Printf.sprintf "/*\n%s\n*/"
53+
let non_hack_prefix = "/*@non_hack:\n"
54+
55+
let non_hack_suffix = "\n*/"
56+
57+
let strip_non_hack_comment (code : string) =
58+
if
59+
String.is_prefix code ~prefix:non_hack_prefix
60+
&& String.is_suffix code ~suffix:non_hack_suffix
61+
then
62+
String.sub
63+
code
64+
~pos:(String.length non_hack_prefix)
65+
~len:
66+
(String.length code
67+
- String.length non_hack_prefix
68+
- String.length non_hack_suffix)
69+
else
70+
code
71+
72+
let wrap_in_comment : string -> string = Printf.sprintf "/*@non_hack:\n%s\n*/"
5473

5574
let to_hack { chunk_kind; id; contents; cell_bento_metadata } : string =
5675
let (cell_type, body) =
@@ -126,7 +145,11 @@ let metadata_of_comment (comment : string) :
126145
Error (Notebook_convert_error.Invalid_input msg)
127146
| metadata -> Ok metadata
128147

129-
let strip_comment_wrapper (hack : string) : string =
148+
(** remove leading `/*\n` and trailing `\n*/`. These were inserted
149+
* by older versions of the conversion script for non-hack cells.
150+
* We also handle the newer format of `/*@non_hack:\n` ... `\n*/`
151+
*)
152+
let strip_legacy_comment_wrapper (hack : string) : string =
130153
let stripped = String.strip hack in
131154
String.sub
132155
stripped
@@ -138,12 +161,13 @@ let of_hack ~(comment : string) (hack : string) :
138161
Result.map
139162
(metadata_of_comment comment)
140163
~f:(fun { metadata_id = id; metadata_cell_type; cell_bento_metadata } ->
164+
let hack = strip_non_hack_comment hack in
141165
if String.equal "code" metadata_cell_type then
142166
{ id; chunk_kind = Hack; contents = hack; cell_bento_metadata }
143167
else
144168
{
145169
id;
146170
chunk_kind = Non_hack { cell_type = metadata_cell_type };
147-
contents = strip_comment_wrapper hack;
171+
contents = strip_legacy_comment_wrapper hack;
148172
cell_bento_metadata;
149173
})

hphp/hack/test/notebook_convert/hack_to_notebook/nb1.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,14 @@
1010
*/
1111
//@bento-cell-end
1212

13+
//@bento-cell:{"cell_bento_metadata":{"language":"python"},"cell_type":"code","id":4}
14+
/*@non_hack:
15+
%%python
16+
17+
print('Hello from python')
18+
*/
19+
//@bento-cell-end
20+
1321
//@bento-cell:{"id": 1, "cell_type": "code"}
1422
class N1234MyClass {
1523
public function hello(): void {

hphp/hack/test/notebook_convert/hack_to_notebook/nb1.php.exp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,13 @@
3030
"metadata":{},
3131
"outputs":[],
3232
"source":["echo \"hi2\";\n","$m->hello();"]
33+
},
34+
{
35+
"cell_type":"code",
36+
"execution_count":null,
37+
"metadata":{"language":"python"},
38+
"outputs":[],
39+
"source":["%%python\n","print('Hello from python')"]
3340
}
3441
],
3542
"metadata":{

hphp/hack/test/notebook_convert/hack_to_notebook/nb11_bad_notebook_number.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
//@bento-notebook:{"notebook_number":"1234","kernelspec":{"display_name":"hack","language":"hack","name":"bento_kernel_hack"}}
44

55
//@bento-cell:{"id": 2, "cell_type": "markdown"}
6-
/*
6+
/*@non_hack:
77
# Check it out
88
99
I am a *markdown* **cell**

hphp/hack/test/notebook_convert/hack_to_notebook/nb2_invalid_metadata.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
//@bento-notebook:{"notebook_number":"N1234","kernelspec":{"display_name":"hack","language":"hack","name":"bento_kernel_hack"}}
33

44
//@bento-cell:{"id": 2, "cell_type": "markdown"}
5-
/*
5+
/*@non_hack:
66
# Check it out
77
88
I am a *markdown* **cell**

hphp/hack/test/notebook_convert/hack_to_notebook/nb3_invalid_metadata.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
//@bento-notebook:{"notebook_number":"N1234","kernelspec":{"display_name":"hack","language":"hack","name":"bento_kernel_hack"}}
33

44
//@bento-cell:{"id": 2, "cell_type": "markdown"}
5-
/*
5+
/*@non_hack:
66
# Check it out
77
88
I am a *markdown* **cell**

hphp/hack/test/notebook_convert/hack_to_notebook/nb4_invalid_metadata.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
//@bento-notebook:{"notebook_number":" i am not valid json
44

55
//@bento-cell:{"id": 2, "cell_type": "markdown"}
6-
/*
6+
/*@non_hack:
77
# Check it out
88
99
I am a *markdown* **cell**

hphp/hack/test/notebook_convert/hack_to_notebook/nb7_missing_end_of_cell_marker_best_effort.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
//@bento-notebook:{"notebook_number":"N1234","kernelspec":{"display_name":"hack","language":"hack","name":"bento_kernel_hack"}}
44

55
//@bento-cell:{"id": 2, "cell_type": "markdown"}
6-
/*
6+
/*@non_hack:
77
# Check it out
88
99
I am a *markdown* **cell**

hphp/hack/test/notebook_convert/hack_to_notebook/nb7_missing_end_of_cell_marker_best_effort.php.exp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
"execution_count":null,
2222
"metadata":{},
2323
"outputs":[],
24-
"source":["# Check it out\n","I am a *markdown* **cell**"]
24+
"source":["non_hack:\n","# Check it out\n","I am a *markdown* **cell**"]
2525
},
2626
{
2727
"cell_type":"code",

hphp/hack/test/notebook_convert/notebook_to_hack/nb1.tc.ipynb.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ class N1234MyClass {}
77
//@bento-cell-end
88

99
//@bento-cell:{"cell_bento_metadata":{},"cell_type":"markdown","id":3}
10-
/*
10+
/*@non_hack:
1111
## Markdown cell (cell 3)
1212
1313
I am a *markdown* **cell**

0 commit comments

Comments
 (0)