On Mon, Jul 10, 2017 at 06:48:25PM BST, Robert Peichaer wrote:
> On Fri, Jul 07, 2017 at 05:47:46AM +0100, Raf Czlonka wrote:
> > Hi all,
> >
> > I've noticed that etc/ksh.kshrc uses both types of command substitution
> > `command` and $(command). The below diff unifies it and uses
> > $(command) notation consistently.
> >
> > While there:
> >
> > - remove ':' (null utility) from the very first line of the file -
> > I *do* understand what it does but it doesn't seem like it's needed
> > at all, unless I'm missing something (as is the case with some idioms)
> > - remove basename(1) invocation and use parameter expansion instead
> > - simplify one if conditional by replacing it with && and grouping
> > commands
>
> Having sent a lot of similar diffs myself, I recommend to only change
> one thing in a diff and not to mix stuff. Better send two diffs with one
> type of change each than one diff with multiple.
Hi Robert,
Thanks for the feedback and OKs.
The other changes seemed like too small a ones to warrant separate diffs.
> > Regards,
> >
> > Raf
> >
> > Index: etc/ksh.kshrc
> > ===================================================================
> > RCS file: /cvs/src/etc/ksh.kshrc,v
> > retrieving revision 1.27
> > diff -u -p -u -r1.27 ksh.kshrc
> > --- etc/ksh.kshrc 14 Sep 2016 18:34:51 -0000 1.27
> > +++ etc/ksh.kshrc 7 Jul 2017 04:38:58 -0000
> > @@ -1,4 +1,3 @@
> > -:
> > # $OpenBSD: ksh.kshrc,v 1.27 2016/09/14 18:34:51 rpe Exp $
> > #
> > # NAME:
> > @@ -39,7 +38,7 @@ case "$-" in
> > 0) PS1S='# ';;
> > esac
> > PS1S=${PS1S:-'$ '}
> > - HOSTNAME=${HOSTNAME:-`uname -n`}
> > + HOSTNAME=${HOSTNAME:-$(uname -n)}
>
> OK
>
> > HOST=${HOSTNAME%%.*}
> >
> > PROMPT="$USER:!$PS1S"
> > @@ -49,8 +48,8 @@ case "$-" in
> > PS1=$PPROMPT
> > # $TTY is the tty we logged in on,
> > # $tty is that which we are in now (might by pty)
> > - tty=`tty`
> > - tty=`basename $tty`
> > + tty=$(tty)
> > + tty=${tty##*/}
>
> OK
>
> > TTY=${TTY:-$tty}
> > # $console is the system console device
> > console=$(sysctl kern.consdev)
> > @@ -74,9 +73,8 @@ case "$-" in
> > xterm*)
> > ILS='\033]1;'; ILE='\007'
> > WLS='\033]2;'; WLE='\007'
> > - if ps -p $PPID -o command | grep -q telnet; then
> > + { ps -p $PPID -o command | grep -q telnet; } &&
> > export TERM=xterms
> > - fi
>
> If at all this would be
>
> ps -p $PPID -o command | grep -q telnet &&
> export TERM=xterms
>
> But I doubt this is any improvement compared to if-then-else.
> In contrast to the installer script, there is no need for a
> terse shell scripting style.
I "automatically" grouped them as to ensure the exit code of the
whole sequence rather than the last command - obviously here I've
done it unnecessarily.
While I tried to remove if-then-fi (I only ever try doing this if there
is no *else*) I could not see the wood for the trees - the above
*can* actually be improved using pgrep(1):
pgrep -P $PPID -q telnet && export TERM=xterms
We not only got rid of if-then-fi, but there's also one process less.
That's surely an improvement! :^)
> > ;;
> > *) ;;
> > esac
> > @@ -117,7 +115,7 @@ case "$-" in
> > alias o='fg %-'
> > alias df='df -k'
> > alias du='du -k'
> > - alias rsize='eval `resize`'
> > + alias rsize='eval $(resize)'
>
> OK
>
> > ;;
> > *) # non-interactive
> > ;;
> > @@ -142,6 +140,6 @@ function pre_path {
> > }
> > # if $1 is in path, remove it
> > function del_path {
> > - no_path $* || eval ${2:-PATH}=`eval echo :'$'${2:-PATH}: |
> > - sed -e "s;:$1:;:;g" -e "s;^:;;" -e "s;:\$;;"`
> > + no_path $* || eval ${2:-PATH}=$(eval echo :'$'${2:-PATH}: |
> > + sed -e "s;:$1:;:;g" -e "s;^:;;" -e "s;:\$;;")
>
> OK
>
> > }
> >
>
> --
> -=[rpe]=-
Best regards,
Raf
