Hi,

On Sun, Jun 19, 2016 at 8:16 AM, Paul Wise wrote:
> On Sun, 2016-06-19 at 00:32 +0530, Satyam Zode wrote:
>
>> I made the changes as you mentioned, please find an attached patches.
>
> Personally, I would suggest that the changes you have separated into
> separate patches per file are a single logical change and as such
> should all be made in the same commit. Some discussion of this "logical
> change" concept from the OpenStack community is here:
>
> https://wiki.openstack.org/wiki/GitCommitMessages
>
Thanks :) Good resource for learning!

>> I have removed this temporarily! we already had discussion regarding
>> this on IRC. Looking forward to your response!
>
> I would suggest leaving the range completers in, even with the enforced
> sorting by bash itself, I think they are useful. For the upper end of
> the completion range, I think go with double the current maximum.
Here is range completion using updated patch:

satyam@satyamz:~/debian/satymaz-guest/copy/diffoscope/bin$ ./diffoscope -
--css                      --help
--max-diff-block-lines     --text
--debug                    --html
--max-diff-input-lines     --version
--debugger                 --html-dir
--max-report-size
--fuzzy-threshold          --jquery                   --new-file
-h                         --list-tools
--separate-file-diff-size
satyam@satyamz:~/debian/satymaz-guest/copy/diffoscope/bin$
./diffoscope --max-report-size
0        1000000  1200000  1400000  1600000  1800000  200000   2000000
 400000   600000   800000
satyam@satyamz:~/debian/satymaz-guest/copy/diffoscope/bin$
./diffoscope --max-diff-block-lines
0   10  15  20  25  30  35  40  45  5   50
satyam@satyamz:~/debian/satymaz-guest/copy/diffoscope/bin$
./diffoscope --max-diff-input-lines
0       100000  20000   30000   40000   5000    55000   65000   75000
 85000   95000
10000   15000   25000   35000   45000   50000   60000   70000   80000   90000
satyam@satyamz:~/debian/satymaz-guest/copy/diffoscope/bin$
./diffoscope --separate-file-diff-size
0       100000  120000  140000  160000  180000  20000   200000  40000
 60000   80000
satyam@satyamz:~/debian/satymaz-guest/copy/diffoscope/bin$
./diffoscope --fuzzy-threshold
0    100  120  140  160  180  20   200  220  240  260  280  300  320
340  360  380  40   400  60   80


>
> Some further comments on the patches below:
>
>> +dh_auto_build:
>> +     debian/diffoscope.bash-completion
>> +     debian/diffoscope.1
>
> This will cause the build to fail, those three lines need to all be on
> one single line with spaces in between. Did gmail wrap it for you?
>
> dh_auto_build: debian/diffoscope.bash-completion debian/diffoscope.1
>
>> +++ b/debian/clean
>> @@ -0,0 +1,3 @@
>> +rm -f debian/diffoscope.1
>> +dh_clean -O--buildsystem=pybuild
>
> The first two lines of the debian/clean file need fixes. See the manual
> page for dh_clean, but the debian/clean file is simply a list of files
> (one per line) to be removed by dh_clean from `debian/rules clean`.
> The full debian/clean file for diffoscope should look like this:
>
> debian/diffoscope.1
> debian/diffoscope.bash-completion
>
>> +    elif '_ARGCOMPLETE' not in os.environ:
>
> This will trigger the error in the circumstance where you don't have
> argcomplete installed and you aren't asking for completion. This means
> it will give an error when running it from the command-line if
> argcomplete isn't installed. I think we want an error when you don't
> have argcomplete installed but you are asking for completion.
> To fix this, remove the "not" from this line.
>
>> +        print('ERROR: Argument completion requested but Python argcomplete 
>> module not installed', file=sys.stderr)
>
> In another place in the diffoscope codebase, a critical error is
> reported using logger.critical before exiting with an error.
> I'm not sure if print, logger.critical or logger.error should be used
> for this. I guess Lunar or other folks can advise you about this.
>
> logger.critical('Console is unable to print Unicode characters. Set e.g. 
> LC_CTYPE=en_US.UTF-8')
>
Here, I used logger.error for displaying error and if error occurs
diffoscope returns.

Thanks!
Satyam Zode
From 849db5cbf37905973e5171239e5222a673117536 Mon Sep 17 00:00:00 2001
From: Satyam Zode <satyamz...@gmail.com>
Date: Sun, 19 Jun 2016 15:14:42 +0530
Subject: [PATCH 2/2] Add dependencies for argument completion

Add new rules for bash-completion script.
Add new file debian/clean.
---
 debian/clean   |  2 ++
 debian/control |  2 ++
 debian/rules   | 13 +++++++------
 3 files changed, 11 insertions(+), 6 deletions(-)
 create mode 100644 debian/clean

diff --git a/debian/clean b/debian/clean
new file mode 100644
index 0000000..81040f5
--- /dev/null
+++ b/debian/clean
@@ -0,0 +1,2 @@
+debian/diffoscope.1
+debian/diffoscope.bash-completion
diff --git a/debian/control b/debian/control
index 19619d7..b24fbef 100644
--- a/debian/control
+++ b/debian/control
@@ -7,9 +7,11 @@ Uploaders:
  Mattia Rizzolo <mat...@debian.org>,
  Reiner Herrmann <rei...@reiner-h.de>,
 Build-Depends:
+ bash-completion,
  binutils-multiarch,
  debhelper (>= 9),
  dh-python,
+ python-argcomplete,
  python3-all,
  python3-debian,
  python3-docutils,
diff --git a/debian/rules b/debian/rules
index b15a73b..f6bd49a 100755
--- a/debian/rules
+++ b/debian/rules
@@ -8,10 +8,10 @@ ifneq ($(VERSION_dch),$(VERSION_py))
 endif
 
 %:
-	dh $@ --with python3 --buildsystem=pybuild
+	dh $@ --with python3 --with bash-completion --buildsystem=pybuild
 
 override_dh_python3:
-	dh_python3 --recommends=python-debian --recommends=rpm-python --recommends=tlsh --recommends=guestfs
+	dh_python3 --recommends=python-debian --recommends=rpm-python --recommends=tlsh --recommends=guestfs --recommends=argcomplete
 
 override_dh_gencontrol:
 	TOOLS="$$(bin/diffoscope --list-tools=debian | tail -n 1 | \
@@ -23,13 +23,14 @@ override_dh_gencontrol:
 debian/diffoscope.1: debian/diffoscope.1.rst
 	rst2man $< $@
 
+debian/diffoscope.bash-completion:
+	register-python-argcomplete diffoscope > $@
+
+dh_auto_build: debian/diffoscope.bash-completion debian/diffoscope.1
+
 override_dh_installman: debian/diffoscope.1
 	dh_installman -O--buildsystem=pybuild
 
-override_dh_clean:
-	rm -f debian/diffoscope.1
-	dh_clean -O--buildsystem=pybuild
-
 diffoscope/presenters/icon.py: favicon.png
 	(echo '# Generated from favicon.png'; \
 	 echo 'FAVICON_BASE64 = """'; \
-- 
2.1.4

From d14b88bd52a617fb39d31ea06386509e357f1b6b Mon Sep 17 00:00:00 2001
From: Satyam Zode <satyamz...@gmail.com>
Date: Sun, 19 Jun 2016 15:09:46 +0530
Subject: [PATCH 1/2] Add argument completion feature to diffoscope

This should enable argument completion for diffoscope.

For argument completion `python-argcomplete` module needs to be installed.

If `python-argcomplete` module is not installed and tab is pressed then
diffoscope returns with status 1.

RangeCompleter class is used as completer for different options having
integer arguments.

Add documentation for argcomplete in README.rst.

Closes-bug: #826711
---
 README.rst             |  4 ++++
 bin/diffoscope         |  1 +
 diffoscope/__main__.py | 32 +++++++++++++++++++++++++++-----
 3 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/README.rst b/README.rst
index 5288e52..a2291d6 100644
--- a/README.rst
+++ b/README.rst
@@ -59,6 +59,10 @@ Optionally, the following modules will enhance it:
   ``python-magic``. It is built from `file
   <http://www.darwinsys.com/file/>`_.
   Available on Debian and Fedora as ``python3-magic``.
+*  ``argcomplete`` is used for argument completion.
+  Available on Debian as ``python3-argcomplete``.
+  Available on Fedora as ``python-argcomplete``.
+  Available on `PyPI <https://pypi.python.org/pypi/argcomplete/>`
 
 The various comparators rely on external commands being available. To
 get a list of them, please run::
diff --git a/bin/diffoscope b/bin/diffoscope
index 2393807..2422b70 100755
--- a/bin/diffoscope
+++ b/bin/diffoscope
@@ -1,4 +1,5 @@
 #!/usr/bin/env python3
+# PYTHON_ARGCOMPLETE_OK
 # -*- coding: utf-8 -*-
 #
 # diffoscope: in-depth comparison of files, archives, and directories
diff --git a/diffoscope/__main__.py b/diffoscope/__main__.py
index ac7913c..725597a 100644
--- a/diffoscope/__main__.py
+++ b/diffoscope/__main__.py
@@ -1,4 +1,5 @@
 #!/usr/bin/env python3
+# PYTHON_ARGCOMPLETE_OK
 # -*- coding: utf-8 -*-
 #
 # diffoscope: in-depth comparison of files, archives, and directories
@@ -30,6 +31,10 @@ try:
     import tlsh
 except ImportError:
     tlsh = None
+try:
+    import argcomplete
+except ImportError:
+    argcomplete = None
 from diffoscope import logger, VERSION, set_locale, clean_all_temp_files
 import diffoscope.comparators
 from diffoscope.config import Config
@@ -60,25 +65,30 @@ def create_parser():
                         dest='max_report_size', type=int,
                         help='maximum bytes written in report (default: %d)' %
                         Config.general.max_report_size,
-                        default=Config.general.max_report_size)
+                        default=Config.general.max_report_size).completer=RangeCompleter(0,
+                        Config.general.max_report_size, 200000)
     parser.add_argument('--separate-file-diff-size', metavar='BYTES',
                         dest='separate_file_diff_size', type=int,
                         help='diff size to load diff on demand, with --html-dir (default: %d)' %
                         Config.general.separate_file_diff_size,
-                        default=Config.general.separate_file_diff_size)
+                        default=Config.general.separate_file_diff_size).completer=RangeCompleter(0,
+                        Config.general.separate_file_diff_size, 20000)
     parser.add_argument('--max-diff-block-lines', dest='max_diff_block_lines', type=int,
                         help='maximum number of lines per diff block (default: %d)' %
                         Config.general.max_diff_block_lines,
-                        default=Config.general.max_diff_block_lines)
+                        default=Config.general.max_diff_block_lines).completer=RangeCompleter(0,
+                        Config.general.max_diff_block_lines, 5)
     parser.add_argument('--max-diff-input-lines', dest='max_diff_input_lines', type=int,
                         help='maximum number of lines fed to diff (default: %d)' %
                         Config.general.max_diff_input_lines,
-                        default=Config.general.max_diff_input_lines)
+                        default=Config.general.max_diff_input_lines).completer=RangeCompleter(0,
+                        Config.general.max_diff_input_lines, 5000)
     parser.add_argument('--fuzzy-threshold', dest='fuzzy_threshold', type=int,
                         help='threshold for fuzzy-matching '
                              '(0 to disable, %d is default, 400 is high fuzziness)' %
                              (Config.general.fuzzy_threshold),
-                        default=Config.general.fuzzy_threshold)
+                        default=Config.general.fuzzy_threshold).completer=RangeCompleter(0,
+                        400, 20)
     parser.add_argument('--new-file', dest='new_file', action='store_true',
                         help='treat absent files as empty')
     parser.add_argument('--css', metavar='url', dest='css_url',
@@ -89,6 +99,12 @@ def create_parser():
     parser.add_argument('file2', help='second file to compare')
     if not tlsh:
         parser.epilog = 'File renaming detection based on fuzzy-matching is currently disabled. It can be enabled by installing the “tlsh” module available at https://github.com/trendmicro/tlsh'
+    if argcomplete:
+        argcomplete.autocomplete(parser)
+    elif '_ARGCOMPLETE' in os.environ:
+        logger.error('Argument completion requested but Python argcomplete module not installed. It can be enabled by installing the  “python-argcomplete” module by running  “pip install argcomplete”')
+        sys.exit(1)
+
     return parser
 
 
@@ -105,6 +121,12 @@ def make_printer(path):
     if path != '-':
         output.close()
 
+class RangeCompleter(object):
+    def __init__(self, start, end, step):
+        self.choices = range(start, end + 1, step)
+
+    def __call__(self, prefix, **kwargs):
+        return (str(i) for i in self.choices if str(i).startswith(prefix))
 
 class ListToolsAction(argparse.Action):
     def __call__(self, parser, namespace, os_override, option_string=None):
-- 
2.1.4

_______________________________________________
Reproducible-builds mailing list
Reproducible-builds@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/reproducible-builds

Reply via email to