Debian Bug report logs - #826711
diffoscope: please add argument completion

Toggle useless messages

Message #5 received at submit@bugs.debian.org (full text, mbox, reply):

From: Paul Wise <pabs@debian.org>

To: Debian Bug Tracking System <submit@bugs.debian.org>

Subject: diffoscope: please add argument completion

Date: Wed, 08 Jun 2016 16:32:33 +0800

[Message part 1 (text/plain, inline)]

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 (application/pgp-signature, inline)]

Message #10 received at 826711@bugs.debian.org (full text, mbox, reply):

From: Satyam Zode <satyamzode@gmail.com>

To: Paul Wise <pabs@debian.org>, 826711@bugs.debian.org, Reproducible Builds discussion list <reproducible-builds@lists.alioth.debian.org>

Cc: diffoscope@lists.reproducible-builds.org

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

Date: Sat, 11 Jun 2016 16:31:53 +0530

[Message part 1 (text/plain, inline)]

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

[0001-Added-support-for-argument-completion.patch (text/x-patch, attachment)]

Message #15 received at 826711@bugs.debian.org (full text, mbox, reply):

From: Paul Wise <pabs@debian.org>

To: Satyam Zode <satyamzode@gmail.com>, 826711@bugs.debian.org, Reproducible Builds discussion list <reproducible-builds@lists.alioth.debian.org>

Cc: diffoscope@lists.reproducible-builds.org

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

Date: Sat, 11 Jun 2016 19:12:47 +0800

[Message part 1 (text/plain, inline)]

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 (application/pgp-signature, inline)]

Message #20 received at 826711@bugs.debian.org (full text, mbox, reply):

From: Paul Wise <pabs@debian.org>

To: Satyam Zode <satyamzode@gmail.com>, 826711@bugs.debian.org, Reproducible Builds discussion list <reproducible-builds@lists.alioth.debian.org>

Cc: diffoscope@lists.reproducible-builds.org

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

Date: Sat, 11 Jun 2016 19:15:25 +0800

[Message part 1 (text/plain, inline)]

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 (application/pgp-signature, inline)]

Message #25 received at 826711@bugs.debian.org (full text, mbox, reply):

From: Satyam Zode <satyamzode@gmail.com>

To: Paul Wise <pabs@debian.org>

Cc: 826711@bugs.debian.org, Reproducible Builds discussion list <reproducible-builds@lists.alioth.debian.org>, diffoscope@lists.reproducible-builds.org

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

Date: Sun, 12 Jun 2016 11:50:28 +0530

[Message part 1 (text/plain, inline)]

Tags: patch


Hi, Pabs!

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

Thanks!
Satyam Zode,

[0001-Added-support-for-argument-completion.patch (text/x-patch, attachment)]

Message #30 received at 826711@bugs.debian.org (full text, mbox, reply):

From: Paul Wise <pabs@debian.org>

To: Satyam Zode <satyamzode@gmail.com>

Cc: 826711@bugs.debian.org, Reproducible Builds discussion list <reproducible-builds@lists.alioth.debian.org>, diffoscope@lists.reproducible-builds.org

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

Date: Sun, 12 Jun 2016 16:01:16 +0800

[Message part 1 (text/plain, inline)]

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

> Tags: patch

That only works in mails to submit@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 <https://pypi.python.org/pypi/argcomplete/>`_.

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

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

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

-- 
bye,
pabs

https://wiki.debian.org/PaulWise

[signature.asc (application/pgp-signature, inline)]

Message #35 received at 826711@bugs.debian.org (full text, mbox, reply):

From: Satyam Zode <satyamzode@gmail.com>

To: Paul Wise <pabs@debian.org>

Cc: 826711@bugs.debian.org, Reproducible Builds discussion list <reproducible-builds@lists.alioth.debian.org>, diffoscope users and developers <diffoscope@lists.reproducible-builds.org>

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

Date: Sun, 19 Jun 2016 00:32:39 +0530

[Message part 1 (text/plain, inline)]

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 <pabs@debian.org> wrote:

> On Sun, 2016-06-12 at 11:50 +0530, Satyam Zode wrote:
>
> > Tags: patch
>
> That only works in mails to submit@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 <https://pypi.python.org/pypi/argcomplete/>`_.
>
>
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, 200000)
>
> I think this prevents people from completing values above the
> default max_report_size? Same for the other RangeCompleters.
>
> Interestingly, in the range completers, 1000000 appears to sort before
> 800000, 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

[Message part 2 (text/html, inline)]

[0004-Added-new-rules-for-bash-completion-script.patch (text/x-patch, attachment)]

[0003-Added-debian-clean-file.patch (text/x-patch, attachment)]

[0002-Added-dependencies-for-argument-completion.patch (text/x-patch, attachment)]

[0001-Added-support-for-argument-completion.patch (text/x-patch, attachment)]

Message #40 received at 826711@bugs.debian.org (full text, mbox, reply):

From: Paul Wise <pabs@debian.org>

To: Satyam Zode <satyamzode@gmail.com>

Cc: 826711@bugs.debian.org, Reproducible Builds discussion list <reproducible-builds@lists.alioth.debian.org>, diffoscope users and developers <diffoscope@lists.reproducible-builds.org>

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

Date: Sun, 19 Jun 2016 10:46:09 +0800

[Message part 1 (text/plain, inline)]

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 (application/pgp-signature, inline)]

Message #45 received at 826711@bugs.debian.org (full text, mbox, reply):

From: Satyam Zode <satyamzode@gmail.com>

To: Paul Wise <pabs@debian.org>

Cc: 826711@bugs.debian.org, Reproducible Builds discussion list <reproducible-builds@lists.alioth.debian.org>, diffoscope users and developers <diffoscope@lists.reproducible-builds.org>

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

Date: Sun, 19 Jun 2016 15:40:07 +0530

[Message part 1 (text/plain, inline)]

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

[0002-Add-dependencies-for-argument-completion.patch (text/x-patch, attachment)]

[0001-Add-argument-completion-feature-to-diffoscope.patch (text/x-patch, attachment)]

Message #50 received at 826711@bugs.debian.org (full text, mbox, reply):

From: Paul Wise <pabs@debian.org>

To: Satyam Zode <satyamzode@gmail.com>

Cc: 826711@bugs.debian.org, Reproducible Builds discussion list <reproducible-builds@lists.alioth.debian.org>, diffoscope users and developers <diffoscope@lists.reproducible-builds.org>

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

Date: Sun, 19 Jun 2016 18:19:12 +0800

[Message part 1 (text/plain, inline)]

On Sun, 2016-06-19 at 15:40 +0530, Satyam Zode wrote:

> Here is range completion using updated patch:
...
> Here, I used logger.error for displaying error and if error occurs
> diffoscope returns.

Excellent, thanks! From my perspective this is ready to be committed.

-- 

bye,
pabs

https://wiki.debian.org/PaulWise

[signature.asc (application/pgp-signature, inline)]

Message #57 received at 826711-close@bugs.debian.org (full text, mbox, reply):

From: Reiner Herrmann <reiner@reiner-h.de>

To: 826711-close@bugs.debian.org

Subject: Bug#826711: fixed in diffoscope 55

Date: Fri, 15 Jul 2016 22:06:24 +0000

Source: diffoscope
Source-Version: 55

We believe that the bug you reported is fixed in the latest version of
diffoscope, which is due to be installed in the Debian FTP archive.

A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 826711@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Reiner Herrmann <reiner@reiner-h.de> (supplier of updated diffoscope package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmaster@ftp-master.debian.org)


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Format: 1.8
Date: Fri, 15 Jul 2016 21:50:29 +0200
Source: diffoscope
Binary: diffoscope
Architecture: source
Version: 55
Distribution: unstable
Urgency: medium
Maintainer: Reproducible builds folks <reproducible-builds@lists.alioth.debian.org>
Changed-By: Reiner Herrmann <reiner@reiner-h.de>
Description:
 diffoscope - in-depth comparison of files, archives, and directories
Closes: 826711 826894 827981 830434
Changes:
 diffoscope (55) unstable; urgency=medium
 .
   [ anthraxx ]
   * Fix sqlite3 magic recognition when using file >= 5.27. (Closes: #830434)
 .
   [ Satyam Zode ]
   * Add argument completion feature to diffoscope. (Closes: #826711)
 .
   [ Chris Lamb ]
   * Improve message when data differs. (Closes: #827981)
 .
   [ Reiner Herrmann ]
   * Clarify the input arguments. (Closes: #826894)
 .
   [ Holger Levsen ]
   * Add link to homepage in HTML output.
 .
   [ Rainer Müller ]
   * python-magic >=0.4.12 does not require decode.
Checksums-Sha1:
 36a0416ed8cf2b041bc03dcd5911786467d2aec4 2010 diffoscope_55.dsc
 d0b1b236d4831549476f15e8455431278a2db70b 288076 diffoscope_55.tar.xz
Checksums-Sha256:
 c78e89a9cff0233fb36cfea463fc6eac9f99a165266c9865f8042fdf9ef0cc13 2010 diffoscope_55.dsc
 05aeee77b67f74c55ab5d4bdacfd69d39acff184349e3e3bee799f600919e398 288076 diffoscope_55.tar.xz
Files:
 ad940ca99a3178b7e25ae193c715ec29 2010 devel optional diffoscope_55.dsc
 785a8646216a14c172b6d8729e5c5eb2 288076 devel optional diffoscope_55.tar.xz

-----BEGIN PGP SIGNATURE-----

iQIcBAEBCgAGBQJXiUEOAAoJEMzwSSjbDuqn/08QAJawM+1ou4WLPdKZ3mrYgVhR
Y1x5bLaOOKQFfXUHgReo4ZxIOL0Tijp0z2RqmUCdN7/wxJXwhI7ZVCJlPsP2fjS4
r3Jd6mMWOoR/zhmLyLW9HaiYwAwKduPMycWcH4NogRXE5szSe2bEsdaVJi2qiUSL
UN24mzPOFPJ7eB6LBnbQU29ait81dIAPP4xcTOXTQJXItYDR94FFHJP8MlA8Fjaj
54iw3E43FWyDRrHD9oi/wik09CNuwfHXcPAY5jRmkieIOH6Bs2hG66P0qVhf26CV
rD+H/A9kXTFEPAH/mkYK8Oj2KiO/Hxp4unqPPvJwPbPlLuY7q3QQyZbYDswKGEP2
RG3RyVg3cMie2zv9N7tC/HfKs5Dbr+303jgaFqxs8plavo8TjtX+5BJ4WpCp9hqe
WhLP4E2R0YuHKYkE44d5iXU2sW9UVXefW9xcvRQx73J2vC+6Neof5CdUwBWogw3w
wDBez8UGNne8WNnkmaGeVmFmuIPZ8cJp9MXJKSIIMeDkza2tXtrKMewOuICc2yAA
+ci18+6TKSzQdl7c1DJSEJJqB/TBUvd2rab5A1uwm2/vaagU8DmJBKr5EAIXSTzT
DGl3J3k2QVcKKLSogBVyQBIo+Ymv+UsxxhvULhK2PqHrEc7QzQ6pBFd1GlTew4xM
zl4wsXWhcnBTO/W8/LVB
=546Q
-----END PGP SIGNATURE-----

Send a report that this bug log contains spam.