[rfc][icedtea-web] Removing applications tab in jawas-about

Omair Majid omajid at redhat.com
Thu Jul 11 08:12:09 PDT 2013


Hi,

On 07/11/2013 10:33 AM, Andrew Azores wrote:
> On 07/10/2013 05:38 PM, Omair Majid wrote:
>> May I suggest adding a one or two line comment to say what this script
>> does and how to run it?
> 
> Also a good idea. I ended up writing a small novel.

Whoops, one more thing I forgot: please add a license header to the file
as well.

>>
>>> +    sed -i -r \
>>> +    -e "s/\t/    /g" ${c: Convert tabs into four spaces}\
>>> +    -e "s/\&/\&/g" ${c: "&" -> "&"}\
>>> +    -e "s/  /\ \ /g" ${c: Double-spaces into HTML
>>> whitespace for formatting}\
>>> +    -e "s/</\&lt;/g" ${c: "<" -> "&lt;"}\
>>> +    -e "s/>/\&gt;/g" ${c: ">" -> "&gt;"}\
>>> +    -e 's_(\&lt;)?(https?://[^ ]*)(\&gt;| |$$)_\1<a
>>> href="\2">\2</a>\3_i' ${c: Creates hyperlinks from http/https URLs}\
>>> +    -e 's/\&lt;(.*@.*)\&gt;/\&lt;<a
>>> href="mailto:\1\?subject=IcedTea-Web">\1<\/a>\&gt;/i' ${c: Create
>>> mailto links from email addresses formatted as <email at example.com>}\
>>> +    -e "s/$/<br>/g" ${c: "\n" -> "<br>"}\
>>> +    "./$FILE"
>> I would have just used multiple sed invocations + bash comments here,
>> instead of the (rather clever) parameter expansion. But it's your call.
> 
> I've gone and done that and it does look cleaner, although now there's a
> "./$FILE" after every line instead of just once, and the -i -r flags
> every time as well. Also the script does have an elapsed time printout
> in "debug" mode and changing it from one monster of a sed invocation to
> several more manageable ones seems to have increased the total runtime
> of the script by about a factor of 3 (although that really means it's
> about a third of a second now rather than a tenth ;-) ). But this is
> still acceptable, I think.

If this is invoked only once during the build and is sub-second, I think
it's acceptable.

> Changelog:
> * Makefile.am (stamps/html-gen): moved plaintext-to-HTML logic into new
> shell script.
> * html-gen.sh: contains plaintext-to-HTML logic previously found in
> Makefile.am. Added a sed expression to cause changelog file listing entries
> to be underlined.
> * AUTHORS: fixed formatting of one line (removed extra space)
> 
> Andrew A

Looks good to me. Please include a hg export'ed patch and I will be
happy to push it for you.

Cheers,
Omair

-- 
PGP Key: 66484681 (http://pgp.mit.edu/)
Fingerprint = F072 555B 0A17 3957 4E95  0056 F286 F14F 6648 4681



More information about the distro-pkg-dev mailing list