Noissue - Add a script for converting Rietveld patches to Git format

patchconv/patchconv.py

 # This file is part of Adblock Plus <https://adblockplus.org/>,
 # Copyright (C) 2006-2016 Eyeo GmbH
 #
 # Adblock Plus is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3 as
 # published by the Free Software Foundation.
 #
 # Adblock Plus is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
 #
 # You should have received a copy of the GNU General Public License
 # along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
 
 import sys
 
+# States of the conversion state machine.
+NORMAL, INDEX, METAINFO = range(3)
+# Double line that SVN uses to separate file name from patch content.
+SVN_SEPARATOR = '=' * 67
+# Template for the line from Git diff which is replaced during SVN
+# conversion.
+GIT_PART_HEAD = 'diff --git a/{} b/{}\n'
+
 
 def rietveld_to_git(lines):
     """Convert patch from Rietveld format to Git format.
 
     Rietveld format looks similar to SVN patch format but it can also
     contain Git extensions if it was produced by `upload.py` run from
     a project managed by Git or Mercurial. The output format is the
     original Git patch as produced by `git diff` or `hg diff --git`.
     It can be applied by `hg import` or `git apply`.
 
     Arguments:
         lines -- lines of the patch.
     Returns:
         Lines of the converted patch.
     """
-    # States of the conversion state machine.
-    NORMAL, INDEX, METAINFO = range(3)
-    # Double line that SVN uses to separate file name from patch content.
-    SVN_SEPARATOR = '=' * 67
-    # Template for the line from Git diff which is replaced during SVN
-    # conversion.
-    GIT_PART_HEAD = 'diff --git a/{} b/{}\n'
-
     state = NORMAL
     new_name = None
 
     for line in lines:
         if state is NORMAL:
             if line.startswith('Index: '):
                 new_name = line[7:].strip('\n')
                 state = INDEX
             else:
                 yield line
@@ skipping 13 matching lines @@
                 # File copied.
                 old_name = line[10:].strip('\n')
                 yield GIT_PART_HEAD.format(old_name, new_name)
             else:
                 # File added or removed or changed.
                 yield GIT_PART_HEAD.format(new_name, new_name)
             yield line
             state = NORMAL
 
 
-def script():

[Sebastian Noack, 2016/05/12 21:44:26]

This seems to be an unconventional name for an entry point function, or is it
indeed common? As far as I can tell people usually call it main()?

Also I wonder whether it makes quickly testing changes unnecessarily complicated
since you cannot run this module as script without installing it first.

[Vasily Kuznetsov, 2016/05/12 23:25:16]

Quickly testing is still pretty easy after `setup.py develop` but you're
probably right that `main` is a more conventional name. Let's make it main().

Done.

[Sebastian Noack, 2016/05/12 23:48:51]

I'm just curious is this common practice? Most Python scripts like this I've
seen so far use if __name__ == '__main__'

[Vasily Kuznetsov, 2016/05/17 15:57:30]

if __name__ == '__main__' allows running the script with `python script.py` or
`python -m package.script`. However, this script is installed via
`console_scripts` entry point in setup.py, so it can be run simply by typing
`patchconv`. With this in mind it seems to me that the other ways of running it
are a bit redundant. If we want more flexibility, I could add `if __name__ ==
'__main__': main()` to the end of this file to enable the other options. Do you
think it would be useful?

[Sebastian Noack, 2016/05/17 16:04:09]

I don't have a strong opinion here.

+def main():
     for line in rietveld_to_git(sys.stdin):
         sys.stdout.write(line)