[Reproducible-builds] Bug#806891: [Reproducible-commits] [diffoscope] 01/01: Multi-file HTML output

[Joachim Breitner]

Hi,

Am Mittwoch, den 02.12.2015, 17:32 +0200 schrieb Jérémy Bobbio:

> 
> > +    parser.add_argument('--jquery', metavar='url', dest='jquery_url',
> > +                        help='link to the jquery url, with --html-dir. By default, a symlink to /usr/share/javascript/jquery/jquery.js is created')
> 
> To make the Suggests a reality, I think it would be better to only
> display “By default, a symlink…” if the file is already present on the
> filesystem. Otherwise, if `--html-dir` is specified, the software should
> exit with an error, asking users to specify `--jquery`.

You want --help output that depends on the system state?

> Related question: is the browser going to visit the subpage if I have
> JavaScript disabled? In that case, one other option is to only add the
> JavaScript code when we have access to jQuery. I know some people don't
> want JavaScript in their browser, so specifying something like
> `--no-jquery` or `--jquery=none` could also be a way to turn it off.

See my other mail, I can make it degrade gracefully. Those people will
have JavaScript disabled, so I don’t see a strong usecase for multi-
page html output with JavaScript completely disabled. Note that --html
is an option for those users.

> > +        # no output desired? print text
> > +        if not parsed_args.text_output and not
> > parsed_args.html_output and not parsed_args.html_output_directory:
> > +            parsed_args.text_output = "-"
> 
> Maybe it would be nicer to write:
> 
>     if not any(parsed_args.text_output, parsed_args.html_output,
> parsed_args.html_output_directory):

✓

> > +def output_unified_diff(print_func, directory, anchor,
> > unified_diff):
> > +    if directory and len(unified_diff) >
> > Config.general.separate_file_diff_size:
> > +        # open a new file for this table
> > +        filename="%s.html" % hashlib.md5(anchor.encode('utf-
> > 8')).hexdigest()
> 
> I'm not entirely sure the anchor as it's working right now will be
> unique… 

Ok, I’ll ditch this and simply hash the unified_diff: There, clashes
will not matter.

> > +        logger.debug('separate html output for diff of %s (size
> > %d)', anchor, len(unified_diff))
> > +        with file_printer(directory, filename) as new_print_func:
> > +            output_unified_diff_table(new_print_func,
> > unified_diff)
> 
> So I think it would be great to crash here instead of overwrite if the file
> aleardy exists. What do you think?

Not with the above change.

Also, I want to be able to re-run diffoscope without having to manually
delete the output directory first.

Greetings,
Joachim

-- 
Joachim "nomeata" Breitner
Debian Developer
  nomeata at debian.org | ICQ# 74513189 | GPG-Keyid: F0FBF51F
  JID: nomeata at joachim-breitner.de | http://people.debian.org/~nomeata

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.alioth.debian.org/pipermail/reproducible-builds/attachments/20151202/3f8f5b3b/attachment.sig>