Hi Padraig,
On 26 Jan 2013, at 21:39, Pádraig Brady <p...@draigbrady.com> wrote:
> Thanks for the review and tips Gary.
Welcome, and likewise for yours :)
> While I noticed some of those I didn't fix because
> those styles/issues were already used (elsewhere) in bootstrap.
Unfortunately so :(
> Addressing them are probably best for a subsequent patch.
Agreed, though practically speaking, I fear that subsequent patch will
not arrive :(
> On 01/26/2013 05:27 AM, Gary V. Vaughan wrote:
>> Here is the version I've added to libtool bootstrap (note, the arguments
>> are reversed on mine because it supports multiple VCS in the same tree):
>>
>> # func_insert_if_absent STR FILE...
>> # ---------------------------------
>> # If STR is not already on a line by itself in each FILE, insert it, at the
>> # start. Entries are inserted at the start of the ignore list to ensure
>> # existing entries starting with ! are not overridden. Such entries
>> # support whilelisting exceptions after a more generic blacklist pattern.
>> # sorting the new contents of the file and replacing $FILE with the result.
>> func_insert_if_absent ()
>> {
>> $debug_cmd
>>
>> str=$1
>> shift
>>
>> for file
>> do
>> test -f "$file" || touch "$file"
>>
>> duplicate_entries=`func_gitignore_entries "$file" |sor t |uniq -d`
>
> s/sor t/sort/
Cut n' paste mangling.
>> test -n "$duplicate_entries" \
>> && func_error "duplicate entries in $file: " $duplicate_entries
>>
>> func_grep_q "$str" "$file" \
>> && func_verbose "inserting '$str' into '$file'"
>
> s/&&/||/ ?
> s/$str/^$str\$/ ?
Agreed on both counts.
> I was wary about using that technique because of
> the possibility of regexp significant characters in $str
> It's probably best to just func_verbose in the { sed condition }
Perhaps. But the chances of a filename with a regex metachar (that would
change the outcome) are vanishingly small for the sake of a simple verbose
output string, and the alternative code to work around that corner case
significantly uglier, IMHO.
>>
>> linesold=`func_gitignore_entries "$file" |wc -l`
>> linesnew=`$bs_echo "$str" \
>> |func_gitignore_entries - "$file" |sort -u |wc -l`
>> test $linesold -eq $linesnew \
>> || sed "1i\\$nl$str$nl" "$file" \
>
> s/sed/sed -i/ ?
> Though you probably can't rely on sed -i so something else would be needed.
Yikes! How did I miss that? Thanks for the catch. Amended to:
test "$linesold" -eq "$linesnew" \
|| { sed "1i\\$nl$str$nl" "$file" >"$file"T && mv "$file"T "$file"; } \
|| func_permissions_error "$file"
> thanks,
> Pádraig.
Cheers,
--
Gary V. Vaughan (gary AT gnu DOT org)
