Darren J Moffat wrote:
> Roland Mainz wrote:
>
> I don't want this to be a flamewar.
I don't want to run into another flamewar either...
[snip]
> Personally if we are going to rewrite webrev from scratch I'd seriously
> consider doing it in something other than shell script.
I was not thinking about a rewrite (using shell scripts doesn't mean it
always has to look like spaghetti code... :-) ), but some things could
be done different with ksh93 (I just did a two minute look over the
script, it may require little bit more thinking how things can be done
better):
- Replace usage of /usr/bin/nawk with /usr/xpg4/bin/awk via setting
${PATH} to "/usr/xpg6/bin:/usr/xpg4/bin/:/usr/bin". As a side-effect
this will do the right stuff on Linux automatically (Linux has no
/usr/xpg[46]/ dirs but /usr/bin/awk is already at a (more or less)
POSIX-compatible level)
- Alternatively the whole awk construct could be replaced with ksh93
functionality, at this point the awk problem would completely go away
(and the complex usage of awk... the "eval $(nawk ...)"-thing is only
understandable with lots of coffee and IMO beggs for a rewrite of that
part)
- Fix "webrev" to recognize sources in non-ASCII encodings+locales (like
ja_JP.PCK or the more popular *.UTF-8 encodings). This requires the
tools in /usr/xpg6/bin:/usr/xpg4/bin/ since those in /usr/bin/ are AFAIK
not fully multibyte capable (offtopic: "cw" needs to forward the "-xcsi"
option properly), some tweaks to "html_quote" and some other small
adjustments.
- Use ksh93 builtin commands whereever possible. This makes usually a
HUGE difference in performace
- Use ksh93's version of "getopts" to handle the arguments (as a
side-effect "webrev --man"/"webrev --troff"/"webrev --nroff" will
generate a manual page for free)
- Think about localising the script (e.g. $"mymessage", not the
/usr/bin/gettext thing). The same could be used to localise the output
(e.g. generate diffs for non-english users (since ksh93 support running
one and the same script in multiple locales+encodings at the same time))
- Fix temporary file usage - "webrev" should honor $TMPDIR and 'trap "rm
-f /tmp/$$.* ; exit" 0 1 2 3 15' looks like it attacks all files
starting with the current process PID instead of only those which were
created by webrev (a solution would be to create a subdir for all
temporary files created by webrev&co. and let the exit trap remove that
directory)
- Make the output more XHTML-like
- Various minor tweaks and cleanup (like replacing "echo" with "print"
("print" should be preferred in ksh scripts since it is guranteed to be
a builtin command), fix excessive usage of "printf" etc.)
> >> I have the fixes from the original submitted included in my version of
> >> webrev that I'm adding Mercurial support to and it works just fine.
> >>
> >> Now personally I like the changes this fix proposed because it puts the
> >> code into the style I would have used originally, ie while do < rather
> >> than cat | while do.
> >>
> >> Did you actually look at what the fixes are ?
> >
> > Yes, but my comment was of a more generic nature. And at some point I am
> > wondering more and more why I am doing all the work with ksh93 on
> > Solaris when some of the more interesting (potential) consumers are kept
> > at a strict "pdksh-compatibilty" level. Or short: It's frustrating.
>
> The most significant changes in that bug are actually about things other
> than ksh and are about finding and using an appropriate awk program
> dealing with aspects of the "portability" to other SCM systems (like no
> SCCS on Linux).
BTW: If you really want to use pdksh make sure that all variables which
may contain characters listed in $IFS are properly quoted. ksh88 didn't
handle this correctly in some places and pdksh's attempts to emulate
this made it even more worse (e.g. it may behave differently depending
where it's used unless you explicitly "${quote}" variables... ;-( ).
----
Bye,
Roland
--
__ . . __
(o.\ \/ /.o) [EMAIL PROTECTED]
\__\/\/__/ MPEG specialist, C&&JAVA&&Sun&&Unix programmer
/O /==\ O\ TEL +49 641 7950090
(;O/ \/ \O;)
_______________________________________________
tools-discuss mailing list
tools-discuss@opensolaris.org
