Re: [wdiff-bugs] [PATCH] diff as input

[Denver Gingerich]

On Thu, Apr 2, 2009 at 3:55 AM, Martin von Gagern
<address@hidden> wrote:
> Martin von Gagern wrote:
>> Hi!
>>
>> There are a lot of tools out there which produce (unified) diff as
>> output. Examples are most revision control tools, like cvs, svn, hg,
>> git, bzr, as well as other tools like quilt. There are cases where I
>> would want to look at the changes using wdiff. Therefore I propose the
>> introduction of a new flag, "-u", telling wdiff that the input is in
>> unified diff format.
>>
>> wdiff could then create temporary files, copying lines starting with '-'
>> only to the left one, those starting with '+' only to the right one, and
>> all other lines to both. The first character should be stripped if it is
>> '-', '+' or ' ' but probably kept if it is '@'. Then the normal
>> operation could process those two input files.
>>
>> The result would of course not be the same as a wdiff of the whole
>> original input files, as the unified diff contains only a limited amount
>> of context information. Instead, the @@ lines separating hunks would
>> still be visible, as would be the details about the files compared.
>> wdiff would operate on those informations as well, which I would
>> consider intended behaviour, but I'm not really sure.
>>
>> Any opinions on this before I start coding?
>
> As I got no reply, I simply created the attached patch. I named the flag
> "-d" resp. "--diff-input", as the mode of a diff can be autodetected, so
> that if a future version of wdiff supports other diff formats as input,
> it could reuse that flag.

This seems reasonable.  Overall, I think it's a very useful patch
(assuming it does what I think it does; I haven't tested yet).

> The patch is written on top of my NonSeekableInput.patch from
> https://savannah.gnu.org/bugs/index.php?25883 which I just posted to
> this list as well. It factors out the creation of a temporary buffer
> backed by a file to a separate function create_temporary_side.
>
> The modification of the Usage description affects translations as well.
> Therefore I included the POT in the patch, to exhibit this change,
> although the file it is auto-generated.

You can leave out the POT file in your patch, since it is
auto-generated.  It makes the patch more manageable/readable.

> I also introduced an error
> message for "Too many file arguments", which was incorrectly reported as
> "Missing file arguments" before, and which is the only thing that can go
> wrong with diff input, as zero arguments should use stdin, I think.

Good catch.  Thanks for fixing that.

> It would be nice to see this patch included in wdiff.

I agree.  Since it is a non-trivial patch, you may need to submit a
copyright assignment statement to the FSF if you haven't already in
order for me to accept the patch.  For single changes, see:

http://git.savannah.gnu.org/cgit/gnulib.git/tree/doc/Copyright/assign.changes.manual

If you'd like to submit an assignment statement for this and future
changes, see:

http://git.savannah.gnu.org/cgit/gnulib.git/tree/doc/Copyright/assign.future.manual

If you haven't submitted a copyright assignment statement and don't
want to (I wouldn't blame you), then I'll see about changing wdiff
from being an FSF-copyrighted project so that I don't need to require
this.  I'm not sure what the standard procedure is for doing this, but
I can look into it.  I may have to do this anyway since there are some
changes I pulled in from the previous maintainer which might not have
their copyright assigned to the FSF.

As I mentioned with your other patch, I'd like to review this patch
more fully but it might take several days until I'm less busy.

> I noticed that
> there are other sources in the wdiff source tree whcih might be intended
> as replacements one day, but as they crash on me, I didn't spend much
> effort to adapt them for diff input as well.

I haven't looked at those extra tools much myself; they were inherited
from a previous maintainer.  It's possible that they do something like
that, but it's probably easier to just use new code, especially when
it's already been written, works (I assume your patch does), and is
easily-supported going forward.

Denver
http://ossguy.com/