Re: [PATCH 0/4] doc-diff: support diffing from/to AsciiDoc(tor)

From: Martin Ågren <martin.agren@gmail.com>
Date: Mar 20, 2019, 20:11 UTC
Message-ID: <CAN0heSr0vVYBL9=xG=sqHFvRGHWVRi+Uzn1kekb6w6ZP6FHdog@mail.gmail.com>
In-Reply-To: <20190319031412.GC6173@sigill.intra.peff.net>

On Tue, 19 Mar 2019 at 04:14, Jeff King <peff@peff.net> wrote:

Show 8 quoted lines

>
> On Sun, Mar 17, 2019 at 07:35:59PM +0100, Martin Ågren wrote:
>
> > I've taught `doc-diff` a few new knobs to support usage like
> >
> >   $ ./doc-diff --from-asciidoc --to-asciidoctor HEAD HEAD
>
> Very nice. All the patches look good to me.

Thank you.

Show 5 quoted lines

> The "from" and "to" variants of the options are a little awkward; these
> are really properties of the actual endpoints. It would be nice if we
> had some fixed syntax that defined the whole state, like:
>
>   ./doc-diff asciidoc:HEAD asciidoctor:HEAD^

Right. I wasn't terribly impressed by the look of the "--from-foo --to-bar" syntax, but at least I felt like I couldn't possibly mess up the handling of it. As opposed to parsing substrings and whatnot.

> or something. But I think as you introduce new options (like the
> header/footer cutting) that syntax would get pretty unwieldy. So
> probably the separate options is the best way forward.

And this too. A few more orthogonal parameters on top and it would get hairy pretty quickly. (At least I got the feeling it would.)

I think that at least --cut-header-footer is pretty useless to apply to only one of the sides (you'd be asking for 4-6 lines to definitely differ, for each and every rendered file), so this particular parameter would perhaps not be useful to bake into your alternative syntax.

Show 5 quoted lines

> > I'd be happy to rework the final
> > patch to `--cut-header` if that's preferred.
>
> I think what's here is fine for now. This is our own internal script, so
> if options become useless later on, we can always cull them.

That's true.

Show 5 quoted lines

> > [2] After [1], the date in the footer is still formatted differently
> >     here. It might be a locale thing, and I tend to shy away from even
> >     trying to understand those. :-/
>
> Yeah, mine too.

Thanks for a data point. So at least it's not just me.

Martin