[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: join with header line support

From: Eric Blake
Subject: Re: join with header line support
Date: Fri, 30 Oct 2009 17:23:28 -0600
User-agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20090812 Thunderbird/ Mnenhy/

Hash: SHA1

According to Assaf Gordon on 10/30/2009 5:02 PM:
> Although the above can be accomplished by using several other utilities
> (cut, head, paste, sed or similar combination), having this feature
> built-in in join makes life a lot easier - especially if I'm joining
> severals files ( using pipes ), or using specific output fields (with
> "-o") - join will thus take care of extracting the right field header
> into the header line.

First off, thanks for taking the time to contribute.  Whether or not this
goes anywhere, and whether or not my email seems like a harsh critique,
you should know that one of the joys of free software is that you were
able to scratch your own itch, and that you can use it whether or not it
gets folded in upstream.  That said...

The bar is very high for adding new options, especially for burning a
short option on something that doesn't have much background.  That doesn't
necessarily mean we are outright refusing your patch, but since you
admitted that this can already be done with standardized tools, it may be
a better use of our time to add an example in the documentation of how to
achieve the same effect (or in the process of writing such documentation,
show us how hairy that construct turned out to be and why it is worth
inlining).  That way, people can use the hairy construct now, even if they
don't have GNU coreutils, rather than waiting several years for your new
convenience feature to propagate to enough machines to be worth assuming
that it might be present without having to manually upgrade coreutils first.

> Comments are welcomed. This patch is released under GPLv3 or later.
> If you're willing to accept this patch, I'll be happy to assign
> copyright to GNU, etc.

You'll need documentation, an addition to the testsuite, mention in the
NEWS file, and so forth, before this patch could be worthy of inclusion
(and that is ignoring the technical issue of whether we want this feature;
for which I am abstaining from giving my opinion at the moment).  All
told, it will amount to a non-trivial patch, so yes, you would need to
start the paperwork process of assigning copyright to the FSF; let us know
if you want to further pursue this route.  The HACKING file in a git
checkout has more details on writing a bulletproof patch.

> @@ -191,6 +196,7 @@ by whitespace.  When FILE1 or FILE2 (not
>   --check-order     check that the input is correctly sorted, even\n\
>                       if all input lines are pairable\n\
>   --nocheck-order   do not check that the input is correctly sorted\n\
> +  --header          treat first line in each file as field header line.\n\

The alignment looks weird here.

> +  if (join_header_lines && seq1.count && seq2.count) +    {

This won't compile.  And even if it did, it doesn't match neighboring
style.  It's hard to review something that isn't even complete.

> +      prjoin(seq1.lines[0], seq2.lines[0]);
> +      prevline[0] = NULL ;

No space before ';'; multiple instances in your patch.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/


reply via email to

[Prev in Thread] Current Thread [Next in Thread]