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

add -x : skipping first line like cpython does #3214

Closed
pmp-p opened this issue Aug 9, 2022 · 3 comments · Fixed by #3299
Closed

add -x : skipping first line like cpython does #3214

pmp-p opened this issue Aug 9, 2022 · 3 comments · Fixed by #3299
Labels
C: parser How we parse code. Or fail to parse it. S: accepted The changes in this design / enhancement issue have been accepted and can be implemented T: enhancement New feature or request

Comments

@pmp-p
Copy link

pmp-p commented Aug 9, 2022

https://docs.python.org/dev/using/cmdline.html#cmdoption-x
allows for non standard shebang on dos and web source files.
Black should allow the exact same option allowing to skip the first line and leave it untouched.

web use case : https://pygame-web.github.io/wiki/pygame-script/

https://pygame-web.github.io/showroom/pygame-scripts/org.pygame.touchpong.html

wget https://pygame-web.github.io/showroom/pygame-scripts/org.pygame.touchpong.html
# assuming pygame is installed
python3 -x org.pygame.touchpong.html
@pmp-p pmp-p added the T: enhancement New feature or request label Aug 9, 2022
@ichard26 ichard26 added C: parser How we parse code. Or fail to parse it. S: needs discussion Needs further hashing out before ready for implementation (on desirability, feasibility, etc.) labels Aug 10, 2022
@aaossa
Copy link
Contributor

aaossa commented Sep 27, 2022

Looks like a good place for this would be the format_file_contents method (black/__init__.py). I'm not sure if the skip_source_first_line argument should be part of the Mode instance or not, but it seems like that would require a couple less changes here. Of course, there's still the need to add the flag where the Click command is defined.

Notice that in the example provided by OP, Black would remove the second line (empty) since it would be considered to be removable (the file would be starting with an empty line). Maybe a better partition would be to parse the file from the second non-empty line? But, seems like Python just skips one line so...

diff --git a/src/black/__init__.py b/src/black/__init__.py
index 5b8c974..a86f07e 100644
--- a/src/black/__init__.py
+++ b/src/black/__init__.py
@@ -894,7 +894,7 @@ def check_stability_and_equivalence(
     assert_stable(src_contents, dst_contents, mode=mode)


-def format_file_contents(src_contents: str, *, fast: bool, mode: Mode) -> FileContent:
+def format_file_contents(src_contents: str, *, fast: bool, mode: Mode, skip_source_first_line: bool = False) -> FileContent:
     """Reformat contents of a file and return new contents.

     If `fast` is False, additionally confirm that the reformatted code is
@@ -903,6 +903,10 @@ def format_file_contents(src_contents: str, *, fast: bool, mode: Mode) -> FileCo
     """
     if not src_contents.strip():
         raise NothingChanged
+    header = sep = ""
+    if skip_source_first_line:
+        header, sep, src_contents = src_contents.partition("\n")

     if mode.is_ipynb:
         dst_contents = format_ipynb_string(src_contents, fast=fast, mode=mode)
@@ -914,6 +918,10 @@ def format_file_contents(src_contents: str, *, fast: bool, mode: Mode) -> FileCo
     if not fast and not mode.is_ipynb:
         # Jupyter notebooks will already have been checked above.
         check_stability_and_equivalence(src_contents, dst_contents, mode=mode)
+
+    src_contents = header + sep + src_contents
+    dst_contents = header + sep + dst_contents
+
     return dst_contents

If maintainers agree with these changes, I could work on a PR in the upcoming days 👌

@JelleZijlstra
Copy link
Collaborator

This seems reasonable to add. It's not something I've ever needed but the use case is convincing, and Python itself already has a similar option.

@aaossa
Copy link
Contributor

aaossa commented Oct 4, 2022

Hi, any chance to get #3299 reviewed? @JelleZijlstra

@ichard26 ichard26 added S: accepted The changes in this design / enhancement issue have been accepted and can be implemented and removed S: needs discussion Needs further hashing out before ready for implementation (on desirability, feasibility, etc.) labels Oct 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: parser How we parse code. Or fail to parse it. S: accepted The changes in this design / enhancement issue have been accepted and can be implemented T: enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants