public inbox for gentoo-dev@lists.gentoo.org
 help / color / mirror / Atom feed
From: Christopher Schwan <cschwan@students.uni-mainz.de>
To: <gentoo-dev@lists.gentoo.org>
Subject: Re: [gentoo-dev] RFC: sed script redundancy
Date: Sun, 29 May 2011 13:49:50 +0200	[thread overview]
Message-ID: <201105291349.50497.cschwan@students.uni-mainz.de> (raw)
In-Reply-To: <20110529110032.GW24801@gentoo.org>

On Sunday 29 May 2011 13:00:32 Fabian Groffen wrote:
> On 29-05-2011 12:44:46 +0200, Christopher Schwan wrote:
> > Thank you for that script. I experimented a bit with it and have a number
> > of corrections and suggestions:
> > 
> > - alias does not work because my_sed is not declared at this stage. I
> > removed the whole alias line because I want to selectively enable my_sed
> > 
> > - oargs must be an array in order to make quoting work:
> >    local oargs=( "${@}" )
> > 
> > - In the ewarn line ${oargs} should be changed to ${nargs[@]} (!?)
> > - is it correct to treat -e and -f alike ? I am not sure about that,
> > because the latter expects a file
> 
> Yes, because (also in your function) you always shift, and assume the
> next argument is there.  Hence, you have two identical cases in your
> script now.  I only distinguised between 1) being able to do something
> (-i) and 2) having a pattern to work with (-e/-f or first non-option
> argument as string pattern).

Ok sorry - I did not express myself clearly. Your script is replacing "-f" 
with "-e" and I wasnt sure if this is correct. Buf of course, they can be 
treated alike:

-e|--expression|-f|--file)
	arg="$1"
	shift
	nargs+=( "${arg}" "$1" )
	hade=yes
	;;

> 
> > - If no "-e" is given, the first non-option argument is treated as the
> > sed- script-expression, therefore I added hade=yes in the if-branch
> 
> That one was missing indeed.  I just quickly wrote the proof of concept
> 
> :)
> :
> > The new function now reads:
> [snip improved function]
> 
> > As you can see, I added support for long-options. However, testing the
> > individual sed commands remains to be done. This could be especially
> > difficult if input is taken from stdin (e.g. in cat foo | sed
> > "s:a:b:g").
> 
> You might be able to detect input is a pipe, and temporarily
> write the input to some file, then perform the sed without the -i
> requirement and remove the temp file after the real sed.

Good idea, thanks!

> 
> > I tested my_sed within our sage ebuild[1]. This ebuild contains 39 sed
> > commands and I was able to spot one useless sed.
> 
> Cool, nice to see you've made it into something useful!
> 
> > [1] https://github.com/cschwan/sage-on-gentoo/blob/master/sci-
> > mathematics/sage/sage-4.7.ebuild



  reply	other threads:[~2011-05-29 11:50 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-20 15:39 [gentoo-dev] RFC: sed script redundancy Jeroen Roovers
2011-05-20 15:56 ` Fabian Groffen
2011-05-21 17:34   ` Jeroen Roovers
2011-05-22 10:50     ` Fabian Groffen
2011-05-29 10:44       ` Christopher Schwan
2011-05-29 11:00         ` Fabian Groffen
2011-05-29 11:49           ` Christopher Schwan [this message]
2011-10-28  1:08 ` [gentoo-dev] " Ryan Hill
2013-06-10  3:47 ` [gentoo-dev] " Jeroen Roovers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201105291349.50497.cschwan@students.uni-mainz.de \
    --to=cschwan@students.uni-mainz.de \
    --cc=gentoo-dev@lists.gentoo.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox