Joachim Breitner:
>     Multi-file HTML output

Really great idea. :) Thanks for the initial patch!

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

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.

> +        # 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, 

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

> +        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?

Lunar                                .''`.                    : :Ⓐ  :  # apt-get install anarchism
                                    `. `'` 

Attachment: signature.asc
Description: Digital signature

Reproducible-builds mailing list

Reply via email to