Re: [Reproducible-builds] Bug#826711: diffoscope: please add argument completion

2016-06-19 Thread Satyam Zode 
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
0100  120  140  160  180  20   200
 40   60   80
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   10  2   3   4   500055000   65000   75000
 85000   95000
1   15000   25000   35000   45000   5   6   7   8   9
satyam@satyamz:~/debian/satymaz-guest/copy/diffoscope/bin$
./diffoscope --separate-file-diff-size
0   10  12  14  16  18  2   20  4
 6   8
satyam@satyamz:~/debian/satymaz-guest/copy/diffoscope/bin$
./diffoscope --fuzzy-threshold
0100  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 
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 000..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 ,

Re: [Reproducible-builds] Bug#826711: diffoscope: please add argument completion

2016-06-18 Thread Paul Wise 
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

> 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.

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')

-- 

bye,
pabs

https://wiki.debian.org/PaulWise


signature.asc
Description: This is a digitally signed message part
___
Reproducible-builds mailing list
Reproducible-builds@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/reproducible-builds

Re: [Reproducible-builds] Bug#826711: diffoscope: please add argument completion

2016-06-18 Thread Satyam Zode 
Hi, all !
Pabs, Thanks for suggestions :)
I made the changes as you mentioned, please find an attached patches.

On Sun, Jun 12, 2016 at 1:31 PM, Paul Wise  wrote:

> On Sun, 2016-06-12 at 11:50 +0530, Satyam Zode wrote:
>
> > Tags: patch
>
> That only works in mails to sub...@bugs.debian.org, for mails to the
> bug report address, you want this instead (-1 means the current bug):
>
> Control: tags -1 + patch
>
> > I have updated the patch as per suggestions please review it.
>
> Review below...
>
> >  override_dh_clean:
> > + debian/diffoscope.bash-completion
>
> This causes the package to fail to build. I think override_dh_clean
> should be removed and both the bash completion script and
> debian/diffoscope.1 added to debian/clean. Moving the existing things
> to the debian/clean file should be done in a separate commit.
>
> >   dh_python3 --recommends=python-debian --recommends=rpm-python
> --recommends=tlsh --recommends=guestfs
>
> This line needs to add --recommends=argcomplete
>
> > +override_dh_auto_build:
> > + register-python-argcomplete diffoscope >
> debian/diffoscope.bash-completion
>
> This disables the upstream build system, you should call dh_auto_build
> after calling register-python-argcomplete, I think this is a better way
> to do it than what is there now:
>
> override_dh_auto_build:
> debian/diffoscope.bash-completion debian/diffoscope.1
>
> debian/diffoscope.bash-completion:
> register-python-argcomplete diffoscope > $@
>
I made above changes and here is build log : http://paste.debian.net/740041/
I am not sure if anything else is needed or not in debian/* files.


>
> You also need to add generated files to .gitignore so they don't get
> accidentally committed to the repository.
>
> > +* ``python-argcomplete`` is used for argument completion.
> > +  Available on Debian and Fedora as
> > +  ``python-argcomplete``.
> > +  ``python-argcomplete`` is also available on `PyPI ` as
> > +  ``argcomplete``.
>
> I think I would write that like this:
>
> * ``argcomplete`` is used for argument completion.
>   Available on Debian as ``python3-argcomplete``.
>   Available on Fedora as ``python-argcomplete``.
>   Available on `PyPI `_.
>
>
I have made these changes too.


> > -default=Config.general.max_report_size)
> >
> +
> default=Config.general.max_report_size).completer=RangeCompleter(0,
> > +Config.general.max_report_size, 20)
>
> I think this prevents people from completing values above the
> default max_report_size? Same for the other RangeCompleters.
>
> Interestingly, in the range completers, 100 appears to sort before
> 80, so it is doing string sorting not numeric sorting.
>

I have removed this temporarily! we already had discussion regarding this
on IRC.
Looking forward to your response!


Thanks!
Satyam Zode
From 43f3ed25ab3138cf353c3a34438cc7b8c8418e83 Mon Sep 17 00:00:00 2001
From: Satyam Zode 
Date: Sat, 18 Jun 2016 22:12:58 +0530
Subject: [PATCH 4/4] Added new rules for bash-completion script

---
 debian/rules | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/debian/rules b/debian/rules
index b15a73b..a4bdf79 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,16 @@ 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 d7192fb92a4fca051bee2b13548878923c34995e Mon Sep 17 00:00:00 2001
From: Satyam Zode 
Date: Sat, 18 Jun 2016 22:05:19 +0530
Subject: [PATCH 3/4] Added debian/clean file

---
 debian/clean | 3 +++
 1 file changed, 3 insertions(+)
 create mode 100644 debian/clean

diff --git a/debian/clean b/debian/clean
new file mode 100644
index 000..6e1cd10
--- /dev/null
+++ b/debian/clean
@@ -0,0 +1,3 @@
+rm -f debian/diffoscope.1
+dh_clean -O--buildsystem=pybuild
+debian/diffoscope.bash-completion
-- 
2.1.4

From

Re: [Reproducible-builds] Bug#826711: diffoscope: please add argument completion

2016-06-12 Thread Paul Wise 
On Sun, 2016-06-12 at 11:50 +0530, Satyam Zode wrote:

> Tags: patch

That only works in mails to sub...@bugs.debian.org, for mails to the
bug report address, you want this instead (-1 means the current bug):

Control: tags -1 + patch

> I have updated the patch as per suggestions please review it.

Review below...

>  override_dh_clean:
> + debian/diffoscope.bash-completion

This causes the package to fail to build. I think override_dh_clean
should be removed and both the bash completion script and
debian/diffoscope.1 added to debian/clean. Moving the existing things
to the debian/clean file should be done in a separate commit.

>   dh_python3 --recommends=python-debian --recommends=rpm-python 
> --recommends=tlsh --recommends=guestfs

This line needs to add --recommends=argcomplete

> +override_dh_auto_build:
> + register-python-argcomplete diffoscope > 
> debian/diffoscope.bash-completion

This disables the upstream build system, you should call dh_auto_build
after calling register-python-argcomplete, I think this is a better way
to do it than what is there now:

override_dh_auto_build: debian/diffoscope.bash-completion debian/diffoscope.1

debian/diffoscope.bash-completion:
register-python-argcomplete diffoscope > $@

You also need to add generated files to .gitignore so they don't get
accidentally committed to the repository.

> +* ``python-argcomplete`` is used for argument completion.
> +  Available on Debian and Fedora as
> +  ``python-argcomplete``.
> +  ``python-argcomplete`` is also available on `PyPI ` as
> +  ``argcomplete``.

I think I would write that like this:

* ``argcomplete`` is used for argument completion.
  Available on Debian as ``python3-argcomplete``.
  Available on Fedora as ``python-argcomplete``.
  Available on `PyPI `_.

> -default=Config.general.max_report_size)
> +
> default=Config.general.max_report_size).completer=RangeCompleter(0,
> +Config.general.max_report_size, 20)

I think this prevents people from completing values above the
default max_report_size? Same for the other RangeCompleters.

Interestingly, in the range completers, 100 appears to sort before
80, so it is doing string sorting not numeric sorting.

-- 
bye,
pabs

https://wiki.debian.org/PaulWise


signature.asc
Description: This is a digitally signed message part
___
Reproducible-builds mailing list
Reproducible-builds@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/reproducible-builds

Re: [Reproducible-builds] Bug#826711: diffoscope: please add argument completion

2016-06-11 Thread Paul Wise 
On Sat, 2016-06-11 at 16:31 +0530, Satyam Zode wrote:

> Please find an attached patch which enables argument completion for
> diffoscope. Looking forward to review and feedback.

I forgot you need to generate and install the bash-completion snippet:

register-python-argcomplete diffoscope > 
debian/diffoscope.bash-completion

and add it to debian/clean

-- 
bye,
pabs

https://wiki.debian.org/PaulWise


signature.asc
Description: This is a digitally signed message part
___
Reproducible-builds mailing list
Reproducible-builds@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/reproducible-builds

Re: [Reproducible-builds] Bug#826711: diffoscope: please add argument completion

2016-06-11 Thread Paul Wise 
On Sat, 2016-06-11 at 16:31 +0530, Satyam Zode wrote:

> Please find an attached patch which enables argument completion for
> diffoscope. Looking forward to review and feedback.

Thanks for the patch.

> +++ b/debian/control
> @@ -20,6 +20,7 @@ Build-Depends:
> + python3-argcomplete,

I think you need it in Depends/Recommends too?
Probably put it in the dh_python3 args.

> +if argcomplete:
> +argcomplete.autocomplete(parser)

I think it should exit with an error when completion is enabled
(_ARGCOMPLETE env var) but the argcomplete module is not installed.

-- 
bye,
pabs

https://wiki.debian.org/PaulWise


signature.asc
Description: This is a digitally signed message part
___
Reproducible-builds mailing list
Reproducible-builds@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/reproducible-builds

Re: [Reproducible-builds] Bug#826711: diffoscope: please add argument completion

2016-06-11 Thread Satyam Zode 
Hi, all !


Paul Wise :
>
> Package: diffoscope
> Severity: wishlist
> Tags: newcomer
>
> Please add argument completion. Using the Python 3 argcomplete module
> makes this relatively simple. This project can be used as an example:
>
> https://anonscm.debian.org/cgit/collab-maint/check-all-the-things.git/

Please find an attached patch which enables argument completion for diffoscope.
Looking forward to review and feedback.

Thanks!
Satyam Zode
From 50dffe772b38355eb673994fdea49d9d74be7cac Mon Sep 17 00:00:00 2001
From: Satyam Zode 
Date: Sat, 11 Jun 2016 16:26:28 +0530
Subject: [PATCH] Added support for argument completion. Closes #826711

---
 bin/diffoscope | 1 +
 debian/control | 1 +
 diffoscope/__main__.py | 7 +++
 3 files changed, 9 insertions(+)

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/debian/control b/debian/control
index 19619d7..733d3be 100644
--- a/debian/control
+++ b/debian/control
@@ -20,6 +20,7 @@ Build-Depends:
  python3-rpm,
  python3-setuptools,
  python3-tlsh (>= 3.4.1),
+ python3-argcomplete,
  vim-common,
 Standards-Version: 3.9.8
 Homepage: https://diffoscope.org/
diff --git a/diffoscope/__main__.py b/diffoscope/__main__.py
index ac7913c..ab3739f 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
@@ -89,6 +94,8 @@ 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)
 return parser
 
 
-- 
1.9.1

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

[Reproducible-builds] Bug#826711: diffoscope: please add argument completion

2016-06-08 Thread Paul Wise 
Package: diffoscope
Severity: wishlist
Tags: newcomer

Please add argument completion. Using the Python 3 argcomplete module
makes this relatively simple. This project can be used as an example:

https://anonscm.debian.org/cgit/collab-maint/check-all-the-things.git/

-- 
bye,
pabs

https://wiki.debian.org/PaulWise


signature.asc
Description: This is a digitally signed message part
___
Reproducible-builds mailing list
Reproducible-builds@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/reproducible-builds