Repository: mwouts/jupytext
Pull request #434 (2020-02-13 22:13:27)

Fix shebang handling for languages with non-# comments
The shebang always starts with the sequence `#!` independent of the comment symbol of the programming language. Add a test with Rust, which uses `//` as the comment symbol.

The problem with the existing code is that it uses the comment symbol in the shebang line. The shebang can only start with `#!` though. This leads to two problems.
When parsing the shebang, when converting from text to notebook format, it uses a fixed offset of two characters to take the executable path ([`line[2:]`](https://github.com/mwouts/jupytext/pull/434/files#diff-e90356d1e1bc0107aa247e815ebc6147R148)), which fails if the comment is not only one character, like the two characters of `//`.
Similarly, when converting from notebook to text, it uses the comment symbol again and writes an invalid shebang.

The new test is mostly copied from the Python version with slight adaptions.

codecov[bot]: # [Codecov](https://codecov.io/gh/mwouts/jupytext/pull/434?src=pr&el=h1) Report
> Merging [#434](https://codecov.io/gh/mwouts/jupytext/pull/434?src=pr&el=desc) into [master](https://codecov.io/gh/mwouts/jupytext/commit/bcf1a7b1fd5bc5f73d6d38460dcd0ce93a4f179e?src=pr&el=desc) will **decrease** coverage by `11.04%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/mwouts/jupytext/pull/434/graphs/tree.svg?width=650&token=a1pNjsiILi&height=150&src=pr)](https://codecov.io/gh/mwouts/jupytext/pull/434?src=pr&el=tree)

```diff
@@             Coverage Diff             @@
##           master     #434       +/-   ##
===========================================
- Coverage   98.94%   87.89%   -11.05%     
===========================================
  Files          75       75               
  Lines        7646     7492      -154     
===========================================
- Hits         7565     6585      -980     
- Misses         81      907      +826
```


| [Impacted Files](https://codecov.io/gh/mwouts/jupytext/pull/434?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [tests/test\_read\_simple\_rust.py](https://codecov.io/gh/mwouts/jupytext/pull/434/diff?src=pr&el=tree#diff-dGVzdHMvdGVzdF9yZWFkX3NpbXBsZV9ydXN0LnB5) | `100% <100%> (ø)` | :arrow_up: |
| [jupytext/header.py](https://codecov.io/gh/mwouts/jupytext/pull/434/diff?src=pr&el=tree#diff-anVweXRleHQvaGVhZGVyLnB5) | `99.23% <100%> (ø)` | :arrow_up: |
| [tests/test\_hide\_remove\_input\_outputs\_rmarkdown.py](https://codecov.io/gh/mwouts/jupytext/pull/434/diff?src=pr&el=tree#diff-dGVzdHMvdGVzdF9oaWRlX3JlbW92ZV9pbnB1dF9vdXRwdXRzX3JtYXJrZG93bi5weQ==) | `28.57% <0%> (-71.43%)` | :arrow_down: |
| [tests/test\_black.py](https://codecov.io/gh/mwouts/jupytext/pull/434/diff?src=pr&el=tree#diff-dGVzdHMvdGVzdF9ibGFjay5weQ==) | `31.53% <0%> (-68.47%)` | :arrow_down: |
| [tests/test\_mirror.py](https://codecov.io/gh/mwouts/jupytext/pull/434/diff?src=pr&el=tree#diff-dGVzdHMvdGVzdF9taXJyb3IucHk=) | `36.52% <0%> (-63.48%)` | :arrow_down: |
| [tests/test\_cell\_metadata.py](https://codecov.io/gh/mwouts/jupytext/pull/434/diff?src=pr&el=tree#diff-dGVzdHMvdGVzdF9jZWxsX21ldGFkYXRhLnB5) | `40.5% <0%> (-59.5%)` | :arrow_down: |
| [tests/test\_read\_simple\_pandoc.py](https://codecov.io/gh/mwouts/jupytext/pull/434/diff?src=pr&el=tree#diff-dGVzdHMvdGVzdF9yZWFkX3NpbXBsZV9wYW5kb2MucHk=) | `44.82% <0%> (-55.18%)` | :arrow_down: |
| [jupytext/pandoc.py](https://codecov.io/gh/mwouts/jupytext/pull/434/diff?src=pr&el=tree#diff-anVweXRleHQvcGFuZG9jLnB5) | `40.81% <0%> (-55.11%)` | :arrow_down: |
| [tests/test\_knitr\_spin.py](https://codecov.io/gh/mwouts/jupytext/pull/434/diff?src=pr&el=tree#diff-dGVzdHMvdGVzdF9rbml0cl9zcGluLnB5) | `45.45% <0%> (-54.55%)` | :arrow_down: |
| [tests/test\_active\_cells.py](https://codecov.io/gh/mwouts/jupytext/pull/434/diff?src=pr&el=tree#diff-dGVzdHMvdGVzdF9hY3RpdmVfY2VsbHMucHk=) | `52.54% <0%> (-47.46%)` | :arrow_down: |
| ... and [31 more](https://codecov.io/gh/mwouts/jupytext/pull/434/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/mwouts/jupytext/pull/434?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/mwouts/jupytext/pull/434?src=pr&el=footer). Last update [bcf1a7b...6f52194](https://codecov.io/gh/mwouts/jupytext/pull/434?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

mwouts: Oh that's interesting! Thank you @jonasbb for pointing this out.

Patch:
diff --git a/jupytext/header.py b/jupytext/header.py
index c23ff725..99e89bb7 100644
--- a/jupytext/header.py
+++ b/jupytext/header.py
@@ -47,7 +47,7 @@ def encoding_and_executable(notebook, metadata, ext):
     jupytext_metadata = metadata.get('jupytext', {})
 
     if comment is not None and 'executable' in jupytext_metadata:
-        lines.append(comment + '!' + jupytext_metadata.pop('executable'))
+        lines.append('#!' + jupytext_metadata.pop('executable'))
 
     if 'encoding' in jupytext_metadata:
         lines.append(jupytext_metadata.pop('encoding'))
@@ -144,7 +144,7 @@ def header_to_metadata_and_cell(lines, header_prefix, ext=None):
     encoding_re = re.compile(r'^[ \t\f]*{}.*?coding[:=][ \t]*([-_.a-zA-Z0-9]+)'.format(comment))
 
     for i, line in enumerate(lines):
-        if i == 0 and line.startswith(comment + '!'):
+        if i == 0 and line.startswith('#!'):
             metadata.setdefault('jupytext', {})['executable'] = line[2:]
             start = i + 1
             continue


Test patch:
diff --git a/tests/test_read_simple_rust.py b/tests/test_read_simple_rust.py
index b8c1b1f6..72dad373 100644
--- a/tests/test_read_simple_rust.py
+++ b/tests/test_read_simple_rust.py
@@ -43,3 +43,26 @@ pub fn fib(x: i32) -> i32 {
 ::std::mem::drop''')
     ]))
     compare(jupytext.writes(nb, 'rs'), text)
+
+
+def test_read_write_script_with_metadata_241(no_jupytext_version_number, rsnb="""#!/usr/bin/env scriptisto
+// ---
+// jupyter:
+//   jupytext:
+//     text_representation:
+//       extension: .rs
+//       format_name: light
+//   kernelspec:
+//     display_name: Rust
+//     language: rust
+//     name: rust
+// ---
+
+let mut a: i32 = 2;
+a += 1;
+"""):
+    nb = jupytext.reads(rsnb, 'rs')
+    assert 'executable' in nb.metadata['jupytext']
+    rsnb2 = jupytext.writes(nb, 'rs')
+
+    compare(rsnb, rsnb2)