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

Satyam Zode satyamzode at gmail.com
Sun Jun 19 12:10:07 CEST 2016


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 at 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 at satyamz:~/debian/satymaz-guest/copy/diffoscope/bin$
./diffoscope --max-report-size
0        1000000  1200000  1400000  1600000  1800000  200000   2000000
 400000   600000   800000
satyam at satyamz:~/debian/satymaz-guest/copy/diffoscope/bin$
./diffoscope --max-diff-block-lines
0   10  15  20  25  30  35  40  45  5   50
satyam at 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 at satyamz:~/debian/satymaz-guest/copy/diffoscope/bin$
./diffoscope --separate-file-diff-size
0       100000  120000  140000  160000  180000  20000   200000  40000
 60000   80000
satyam at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Add-dependencies-for-argument-completion.patch
Type: text/x-patch
Size: 2234 bytes
Desc: not available
URL: <http://lists.reproducible-builds.org/pipermail/diffoscope/attachments/20160619/ccbcb162/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-argument-completion-feature-to-diffoscope.patch
Type: text/x-patch
Size: 6122 bytes
Desc: not available
URL: <http://lists.reproducible-builds.org/pipermail/diffoscope/attachments/20160619/ccbcb162/attachment-0001.bin>


More information about the diffoscope mailing list