Don't generate completions if we already have bespoke completions in the data directory

Fixes https://github.com/fish-shell/fish-shell/issues/148
Also fix some Python3 issues

3 files changed, 165 insertions, 59 deletions

diff --git a/share/functions/fish_update_completions.fish b/share/functions/fish_update_completions.fish
index 2d640b0c..9418affd 100644
--- a/share/functions/fish_update_completions.fish
+++ b/share/functions/fish_update_completions.fish
@@ -1,3 +1,3 @@
 function fish_update_completions --description "Update man-page based completions"
-	eval $__fish_datadir/tools/create_manpage_completions.py --manpath --progress
+	eval $__fish_datadir/tools/create_manpage_completions.py --manpath --progress --yield-to $__fish_datadir/completions/
 end
diff --git a/share/tools/create_manpage_completions.py b/share/tools/create_manpage_completions.py
index 051a3a50..f3f4a78b 100755
--- a/share/tools/create_manpage_completions.py
+++ b/share/tools/create_manpage_completions.py
@@ -20,9 +20,15 @@ THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
 import string, sys, re, os.path, gzip, traceback, getopt, errno
 from deroff import Deroffer
 
+# Whether we're Python 3
+IS_PY3 = sys.version_info[0] >= 3
+
 # This gets set to the name of the command that we are currently executing
 CMDNAME = ""
 
+# Information used to track which of our parsers were successful
+PARSER_INFO = {}
+
 # builtcommand writes into this global variable, yuck
 built_command_output = []
 
@@ -34,8 +40,8 @@ diagnostic_indent = 0
 VERY_VERBOSE, BRIEF_VERBOSE, NOT_VERBOSE = 2, 1, 0
 
 # Pick some reasonable default values for settings
-global VERBOSITY, WRITE_TO_STDOUT
-VERBOSITY, WRITE_TO_STDOUT = NOT_VERBOSE, False
+global VERBOSITY, WRITE_TO_STDOUT, DEROFF_ONLY
+VERBOSITY, WRITE_TO_STDOUT, DEROFF_ONLY = NOT_VERBOSE, False, False
 
 def add_diagnostic(dgn, msg_verbosity = VERY_VERBOSE):
     # Add a diagnostic message, if msg_verbosity <= VERBOSITY
@@ -606,30 +612,35 @@ class TypeDeroffManParser(ManParser):
                 
     def name(self):
         return "Deroffing man parser"
+        
+# Return whether the file at the given path is overwritable
+# Raises IOError if it cannot be opened
+def file_is_overwritable(path):
+    result = False
+    file = open(path, 'r')
+    for line in file:
+        # Skip leading empty lines
+        line = line.strip()
+        if not line:
+            continue
+        
+        # We look in the initial run of lines that start with #
+        if not line.startswith('#'):
+            break
+            
+        # See if this contains the magic word
+        if 'Autogenerated' in line:
+            result = True
+            break
+            
+    file.close()
+    return result
+
 
 # Return whether the file at the given path either does not exist, or exists but appears to be a file we output (and hence can overwrite)
 def file_missing_or_overwritable(path):
     try:
-        result = False
-        file = open(path, 'r')
-        for line in file:
-            # Skip leading empty lines
-            line = line.strip()
-            if not line:
-                continue
-            
-            # We look in the initial run of lines that start with #
-            if not line.startswith('#'):
-                break
-                
-            # See if this contains the magic word
-            if 'Autogenerated' in line:
-                result = True
-                break
-                
-        file.close()
-        return result
-            
+        return file_is_overwritable(path)
     except IOError as err:
         if err.errno == 2:
             # File does not exist, full steam ahead
@@ -637,11 +648,16 @@ def file_missing_or_overwritable(path):
         else:
             # Something else happened
             return False
-        
-        
     
+# Delete the file if it is autogenerated
+def cleanup_autogenerated_file(path):
+    try:
+        if file_is_overwritable(path):
+            os.remove(path)
+    except (OSError, IOError):
+        pass
 
-def parse_manpage_at_path(manpage_path, output_directory):
+def parse_manpage_at_path(manpage_path, yield_to_dirs, output_directory):
     filename = os.path.basename(manpage_path)
 
     # Clear diagnostics
@@ -649,23 +665,19 @@ def parse_manpage_at_path(manpage_path, output_directory):
     diagnostic_output[:] = []
     diagnostic_indent = 0
 
-    # Get the "base" command, e.g. gcc.1.gz -> gcc
-    global CMDNAME
-    CMDNAME = filename.split('.', 1)[0]
-
     # Set up some diagnostics
     add_diagnostic('Considering ' + manpage_path)
     diagnostic_indent += 1
     
     if manpage_path.endswith('.gz'):
         fd = gzip.open(manpage_path, 'r')
-    else:
-        fd = open(manpage_path, 'r')
-
-    try: #Utf-8 python3
         manpage = fd.read()
-    except: #Latin-1 python3
-        fd = open(manpage_path, 'r', encoding='latin-1')
+        if IS_PY3: manpage = manpage.decode('latin-1')
+    else:
+        if IS_PY3:
+            fd = open(manpage_path, 'r', encoding='latin-1')
+        else:
+            fd = open(manpage_path, 'r')
         manpage = fd.read()
     fd.close()
 
@@ -689,26 +701,32 @@ def parse_manpage_at_path(manpage_path, output_directory):
     
     # Clear the output list
     built_command_output[:] = []
-        
-    parsers = [Type1ManParser(), Type2ManParser(), Type4ManParser(), Type3ManParser(), TypeDarwinManParser(), TypeDeroffManParser()]
+    
+    if DEROFF_ONLY:
+        parsers = [TypeDeroffManParser()]
+    else:
+        parsers = [Type1ManParser(), Type2ManParser(), Type4ManParser(), Type3ManParser(), TypeDarwinManParser(), TypeDeroffManParser()]
     parsersToTry = [p for p in parsers if p.isMyType(manpage)]
-        
+    
     success = False
     if not parsersToTry:
         add_diagnostic(manpage_path + ": Not supported")
     else:
         for parser in parsersToTry:
-            add_diagnostic('Trying parser ' + parser.name())
+            parser_name = parser.name()
+            add_diagnostic('Trying parser ' + parser_name)
             diagnostic_indent += 1
             success = parser.parseManPage(manpage)
             diagnostic_indent -= 1
-            if success: break
+            if success:
+                PARSER_INFO.setdefault(parser_name, []).append(CMDNAME)
+                break
         
         if success:
             if WRITE_TO_STDOUT:
                 output_file = sys.stdout
             else:
-                fullpath = output_directory + CMDNAME + '.fish'
+                fullpath = os.path.join(output_directory, CMDNAME + '.fish')
                 try:
                     if file_missing_or_overwritable(fullpath):
                         output_file = open(fullpath, 'w')
@@ -736,8 +754,40 @@ def parse_manpage_at_path(manpage_path, output_directory):
             add_diagnostic('%s contains no options or is unparsable (tried parser %s)' % (manpage_path, parser_names), BRIEF_VERBOSE)
     return success
 
-def parse_and_output_man_pages(paths, output_directory, show_progress):
-    global diagnostic_indent
+# Indicates whether the given filename has a presence in one of the yield-to directories
+# If so, there's a bespoke completion and we should not generate one
+def file_in_yield_directory(filename, yield_to_dirs):
+    for yield_dir in yield_to_dirs:
+        test_path = os.path.join(yield_dir, filename)
+        if os.path.isfile(test_path):
+            # Yield to the existing file
+            return True
+    return False
+
+# Indicates whether we want to skip this command because it already had a non-autogenerated completion
+def should_skip_man_page(output_path, filename, yield_to_dirs):
+    # No reason to skip if we're writing to stdout
+    if WRITE_TO_STDOUT:
+        return false
+        
+    # Check all the yield directories
+    for yield_dir in yield_to_dirs:
+        test_path = os.path.join(yield_dir, filename)
+        if os.path.isfile(test_path):
+            # Yield to the existing file
+            return true
+        
+    # See if there's a hand-written file already
+    if not file_missing_or_overwritable(output_path):
+        return true
+    
+    # We made it through, so don't skip
+    return false
+            
+    
+
+def parse_and_output_man_pages(paths, output_directory, yield_to_dirs, show_progress):
+    global diagnostic_indent, CMDNAME
     paths.sort()
     total_count = len(paths)
     successful_count, index = 0, 0
@@ -747,16 +797,41 @@ def parse_and_output_man_pages(paths, output_directory, show_progress):
         print("Parsing man pages and writing completions to {0}".format(output_directory))
     for manpage_path in paths:
         index += 1
+        
+        # Get the "base" command, e.g. gcc.1.gz -> gcc
+        man_file_name = os.path.basename(manpage_path)
+        CMDNAME = man_file_name.split('.', 1)[0]
+        output_file_name = CMDNAME + '.fish'
+        
+        # Show progress if we're doing that
         if show_progress:
-            filename = os.path.basename(manpage_path).split('.', 1)[0]
-            progress_str = '  {0} / {1} : {2}'.format((str(index).rjust(padding_len)), total_count, filename)
+            progress_str = '  {0} / {1} : {2}'.format((str(index).rjust(padding_len)), total_count, man_file_name)
             # Pad on the right with spaces so we overwrite whatever we wrote last time
             padded_progress_str = progress_str.ljust(last_progress_string_length)
             last_progress_string_length = len(progress_str)
             sys.stdout.write("\r{0} {1}\r".format(padded_progress_str, chr(27)))
-            sys.stdout.flush();
+            sys.stdout.flush()
+                        
+        # Maybe we want to skip this item
+        skip = False
+        if not WRITE_TO_STDOUT:
+            # Compute the path that we would write to
+            output_path = os.path.join(output_directory, output_file_name)
+
+            if file_in_yield_directory(output_file_name, yield_to_dirs):
+                # We're duplicating a bespoke completion - delete any existing completion
+                skip = True
+                cleanup_autogenerated_file(output_path)
+            elif not file_missing_or_overwritable(output_path):
+                # Don't overwrite a user-created completion
+                skip = True
+
+        # Now skip if requested
+        if skip:
+            continue
+        
         try:
-            if parse_manpage_at_path(manpage_path, output_directory):
+            if parse_manpage_at_path(manpage_path, yield_to_dirs, output_directory):
                 successful_count += 1
         except IOError:
             diagnostic_indent = 0
@@ -799,9 +874,10 @@ def usage(script_name):
     print("Usage: {0} [-v, --verbose] [-s, --stdout] [-d, --directory] [-p, --progress] files...".format(script_name))
     print("""Command options are:
      -h, --help\t\tShow this help message
-     -v, --verbose\tShow debugging output to stderr
+     -v, --verbose [0, 1, 2]\tShow debugging output to stderr. Larger is more verbose.
      -s, --stdout\tWrite all completions to stdout (trumps the --directory option)
-     -d, --directory\tWrite all completions to the given directory, instead of to ~/.config/fish/completions
+     -d, --directory [dir]\tWrite all completions to the given directory, instead of to ~/.config/fish/completions
+     -y, --yield-to [dir]\tSkip completions that are already present in the given directory
      -m, --manpath\tProcess all man1 files available in the manpath (as determined by manpath)
      -p, --progress\tShow progress
     """)
@@ -809,17 +885,21 @@ def usage(script_name):
 if __name__ == "__main__":
     script_name = sys.argv[0]
     try:
-        opts, file_paths = getopt.gnu_getopt(sys.argv[1:], 'vsd:hmp', ['verbose', 'stdout', 'directory=', 'help', 'manpath', 'progress'])
+        opts, file_paths = getopt.gnu_getopt(sys.argv[1:], 'v:sd:hmpy:z', ['verbose=', 'stdout', 'directory=', 'help', 'manpath', 'progress', 'yield-to='])
     except getopt.GetoptError as err:
         print(err.strerror) # will print something like "option -a not recognized"
         usage(script_name)
         sys.exit(2)
     
+    # If a completion already exists in one of the yield-to directories, then don't overwrite it
+    # And even delete an existing autogenerated one
+    yield_to_dirs = []
+    
     use_manpath, show_progress, custom_dir = False, False, False
     output_directory = ''
     for opt, value in opts:
         if opt in ('-v', '--verbose'):
-            VERBOSE = True
+            VERBOSITY = int(value)
         elif opt in ('-s', '--stdout'):
             WRITE_TO_STDOUT = True
         elif opt in ('-d', '--directory'):
@@ -831,6 +911,12 @@ if __name__ == "__main__":
             use_manpath = True
         elif opt in ('-p', '--progress'):
             show_progress = True
+        elif opt in ('-y', '--yield-to'):
+            yield_to_dirs.append(value)
+            if not os.path.isdir(value):
+                sys.stderr.write("Warning: yield-to directory does not exist: '{0}'\n".format(value))
+        elif opt in ('-z'):
+            DEROFF_ONLY = True
         else:
             assert False, "unhandled option"
     
@@ -851,12 +937,19 @@ if __name__ == "__main__":
         except OSError as e:
             if e.errno != errno.EEXIST:
                 raise
-    
+        
     if True:
-        parse_and_output_man_pages(file_paths, output_directory, show_progress)
+        parse_and_output_man_pages(file_paths, output_directory, yield_to_dirs, show_progress)
     else:
         # Profiling code
         import cProfile, pstats
-        cProfile.run('parse_and_output_man_pages(file_paths, output_directory, show_progress)', 'fooprof')
+        cProfile.run('parse_and_output_man_pages(file_paths, output_directory, yield_to_dirs, show_progress)', 'fooprof')
         p = pstats.Stats('fooprof')
         p.sort_stats('cumulative').print_stats(100)
+    
+    # Here we can write out all the parser infos
+    if False:
+        for name in PARSER_INFO:
+            print('Parser ' + name + ':')
+            print('\t' + ', '.join(PARSER_INFO[name]))
+            print('')
\ No newline at end of file
diff --git a/share/tools/deroff.py b/share/tools/deroff.py
index 980742ff..eb8b0d6c 100755
--- a/share/tools/deroff.py
+++ b/share/tools/deroff.py
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 # -*- coding: utf-8 -*-
 
 """ Deroff.py, ported to Python from the venerable deroff.c """
@@ -6,6 +6,8 @@
 
 import sys, re, string
 
+IS_PY3 = sys.version_info[0] >= 3
+
 class Deroffer:
 
     g_specs_specletter = {
@@ -307,6 +309,7 @@ class Deroffer:
         self.skiplists = False
         self.ignore_sonx = False
         self.output = []
+        self.name = ''
         
         self.OPTIONS = 0
         self.FORMAT = 1
@@ -330,6 +333,7 @@ class Deroffer:
                 'RB': Deroffer.macro_i_ir,
                 'RI': Deroffer.macro_i_ir,
                 'AB': Deroffer.macro_i_ir,
+                'Nm': Deroffer.macro_Nm,
                 '] ': Deroffer.macro_close_bracket,
                 'PS': Deroffer.macro_ps,
                 'PE': Deroffer.macro_pe,
@@ -678,6 +682,13 @@ class Deroffer:
         pass
         return False
         
+    def macro_Nm(self):
+        if self.s == 'Nm\n':
+            self.condputs(self.name)
+        else:
+            self.name = self.s[3:].strip() + ' '
+        return True
+               
     def macro_close_bracket(self):
         self.refer = False
         return False
@@ -1062,12 +1073,14 @@ class Deroffer:
 
 def deroff_files(files):
     for arg in files:
-        print >> sys.stderr, arg
+        sys.stderr.write(arg + '\n')
         if arg.endswith('.gz'):
             f = gzip.open(arg, 'r')
+            str = f.read()
+            if IS_PY3: str = str.decode('latin-1')
         else:
             f = open(arg, 'r')
-        str = f.read()
+            str = f.read()
         d = Deroffer()
         d.deroff(str)
         d.flush_output(sys.stdout)
@@ -1078,7 +1091,7 @@ def deroff_files(files):
 if __name__ == "__main__":
     import gzip
     paths = sys.argv[1:]
-    if False:
+    if True:
         deroff_files(paths)
     else:
         import cProfile, profile, pstats