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

[Paul Wise]

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

> Tags: patch

That only works in mails to submit at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.reproducible-builds.org/pipermail/diffoscope/attachments/20160612/fda4d6d0/attachment.sig>