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 <[email protected]>
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 <[email protected]>,
Reiner Herrmann <[email protected]>,
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 <[email protected]>
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
