[arch-projects] [initscripts] [PATCH 04/12] functions: fix indentation
Tom Gundersen
teg at jklm.no
Sat Jun 25 15:10:10 EDT 2011
On Jun 25, 2011, at 20:11, Dave Reisner <d at falconindy.com> wrote:
> On Sat, Jun 25, 2011 at 12:15:56PM +0200, Kurt J. Bosch wrote:
>> ---
>> functions | 50 +++++++++++++++++++++++++-------------------------
>> 1 files changed, 25 insertions(+), 25 deletions(-)
>>
>> diff --git a/functions b/functions
>> index 61cf3dd..9c881c1 100644
>> --- a/functions
>> +++ b/functions
>> @@ -285,17 +285,17 @@ kill_everything() {
>> # Terminate all processes
>> stat_busy "Sending SIGTERM To Processes"
>> run_hook "$1_prekillall"
>> - local pid k5args=""
>> - for pid in ${omit_pids[@]}; do
>> - k5args+=" -o $pid"
>> - done
>> - /sbin/killall5 -15 $k5args &>/dev/null
>> - /bin/sleep 5
>> + local pid k5args=""
>> + for pid in ${omit_pids[@]}; do
>> + k5args+=" -o $pid"
>> + done
>
> I'd rather just see this whole chunk of code go away. Building the extra
> variable is moot when you can just use a PE:
>
> killall 5 -15 ${omit_pids[@]/#/-o }
>
> d
>
>> + /sbin/killall5 -15 $k5args &>/dev/null
>> + /bin/sleep 5
>> stat_done
>>
>> stat_busy "Sending SIGKILL To Processes"
>> - /sbin/killall5 -9 $k5args &>/dev/null
>> - /bin/sleep 1
>> + /sbin/killall5 -9 $k5args &>/dev/null
>> + /bin/sleep 1
>> stat_done
>>
>> run_hook "$1_postkillall"
>> @@ -371,13 +371,13 @@ NETFS="nonfs,nonfs4,nosmbfs,nocifs,nocodafs,noncpfs,nosysfs,noshfs,nofuse,nofuse
>> # Check local filesystems
>> fsck_all() {
>> stat_busy "Checking Filesystems"
>> - FSCK_OUT=/dev/stdout
>> - FSCK_ERR=/dev/stdout
>> - FSCK_FD=
>> - FORCEFSCK=
>> - [[ -f /forcefsck ]] || in_array forcefsck $(< /proc/cmdline) && FORCEFSCK="-- -f"
>> - run_hook sysinit_prefsck
>> - fsck -A -T -C$FSCK_FD -a -t "$NETFS,noopts=_netdev" $FORCEFSCK >$FSCK_OUT 2>$FSCK_ERR
>> + FSCK_OUT=/dev/stdout
>> + FSCK_ERR=/dev/stdout
>> + FSCK_FD=
>> + FORCEFSCK=
>> + [[ -f /forcefsck ]] || in_array forcefsck $(< /proc/cmdline) && FORCEFSCK="-- -f"
>> + run_hook sysinit_prefsck
>> + fsck -A -T -C$FSCK_FD -a -t "$NETFS,noopts=_netdev" $FORCEFSCK >$FSCK_OUT 2>$FSCK_ERR
>> local fsckret=$?
>> if (( fsckret > 1 )); then
>> stat_fail
>> @@ -426,9 +426,9 @@ fsck_reboot() {
>>
>> mount_all() {
>> stat_busy "Mounting Local Filesystems"
>> - run_hook sysinit_premount
>> - mount -a -t $NETFS -O no_netdev
>> - run_hook sysinit_postmount
>> + run_hook sysinit_premount
>> + mount -a -t $NETFS -O no_netdev
>> + run_hook sysinit_postmount
>> stat_done
>> }
>>
>> @@ -502,13 +502,13 @@ fi
>> set_consolefont() {
>> [[ $CONSOLEFONT ]] || return 0
>> stat_busy "Loading Console Font: $CONSOLEFONT"
>> - #CONSOLEMAP in UTF-8 shouldn't be used
>> - [[ $CONSOLEMAP && ${LOCALE,,} =~ utf ]] && CONSOLEMAP=""
>> - local i
>> - for i in /dev/tty[0-9]*; do
>> - /usr/bin/setfont ${CONSOLEMAP:+-m ${CONSOLEMAP}} \
>> - $CONSOLEFONT -C ${i} &>/dev/null
>> - done
>> + #CONSOLEMAP in UTF-8 shouldn't be used
>> + [[ $CONSOLEMAP && ${LOCALE,,} =~ utf ]] && CONSOLEMAP=""
>> + local i
>> + for i in /dev/tty[0-9]*; do
>> + /usr/bin/setfont ${CONSOLEMAP:+-m ${CONSOLEMAP}} \
>> + $CONSOLEFONT -C ${i} &>/dev/null
>> + done
>> if (( $? )); then
>> stat_fail
>> elif [[ $CONSOLEMAP ]]; then
>> --
>> 1.7.1
>>
>
> I never really understood why we indented like this. Since we seem to
> be split 50/50, I'd argue that it might be better to just separate the
> code with whitespace, e.g.
>
> # more code above here...
>
> stat_busy
> foo
> bar
> baz
> stat_done
>
> # more code below here...
>
> This way, we don't have to worry about what happens when there could be
> a stat_fail somewhere in between that would make us have to wonder wtf
> to do with indenting.
>
> d
The reason for the indentation was to make it obvious how stat_{busy,fail,done} match up. There were some errors due to mismatches before.
I think it is relatively clear how to DTRT: if stat_fail/stat_done is one indentation level below stat_busy, then unindent it. Otherwise (e.g., if it is inside a conditional), don't.
That said, I'm wondering if it wouldn't be clearer if we just used status, and when needed put these code blocks inside functions.
-t
More information about the arch-projects
mailing list