#8852 - ldap sync output filter

edit.sh

-#!/bin/bash

[f.lopez, 2018/05/04 01:50:54]

please use /bin/sh instead of /bin/bash

This way it is portable across POSIX systems.
(only if you make the code POSIX complaint of course)

+#!/bin/sh
 

[f.lopez, 2018/05/04 01:50:54]

What is this script about? What is the intention of the script? Please document
why are you writing this script so I can understand why this code matters.

-IFS=$'\n' lines=($(cat /dev/stdin))

[f.lopez, 2018/05/04 01:50:54]

you may want to look into doing this with `read(2)`

+# Filter out uninformative output lines from the ldap-sync cron job script
 
-for (( i=0; i < ${#lines[*]}; i++ ))

[f.lopez, 2018/05/04 01:50:54]

and while you are looking into the comment above, also look into the other loop
you can do here (hint: while loop), what is the difference between them both and
why would you use one or the other

-do
-  if [[ ${lines[$i]} != *"Updating"* ]] && [[ ${lines[$i]} != *"Found"* ]] && [[ ${lines[$i]} != *"Synchronizing"* ]] && [[ ${lines[$i]} != "#"* ]]; then

[f.lopez, 2018/05/04 01:50:54]

the double square brackets (`[[`) is a bash feature
(http://tiswww.case.edu/php/chet/bash/bashref.html#SEC31)
as I said before, we aim to make shell scripts POSIX complaint, so it can run in
any POSIX system.

then there is this bit "${lines[$i]}" which is not very intuitive IMO, due the
way the array is constructed, there are easier and more maintainable ways of
doing this.

the string comparison can be done in a different way too, with other shell
commands, for instance, have you looked into `grep(1)`?
(tip: look into its manual page)

Always go for 80 (or 79) columns width, this way the code is easier to read,
fits nicely into terminals (look for the default value on your linux terminal or
the manual pages for example), the code doesn't wrap when you have a smaller
screen... there are many reasons to aim for this width.

We also aim for not modifying lines that have nothing to do with the change that
you might do in the future, so, using your same line as an example, you can do
something like:

```
if [[ ${lines[$i]} != *"Updating"* ]] && \
   [[ ${lines[$i]} != *"Found"* ]] && \
   [[ ${lines[$i]} != *"Synchronizing"* ]] && \
   [[ ${lines[$i]} != "#"* ]] \
then
```
I didn't test this, but my point is, if you then need to add another line, or
modify the string to compare, you only modify that exact line and not the whole
conditional so it is easier to see, also the code is more readable in the end
(see, even the 80 columns rule is respected here)

-    echo ${lines[$i]} 
-  elif [[ ${lines[$i]} == *"Updating"* ]] && [[ ${lines[$i+1]} == *"->"* ]]; then
-    echo ${lines[$i]}
-  fi
+last_action=""
+
+while read line; do
+  case "$line" in
+    *-\>*)
+      if [ ! -z "$last_action" ]
+      then
+        echo "$last_action"
+      fi
+
+      echo "$line"
+      last_action=""
+      ;;
+    --\ Found\ *\ users* | \
+    \#* | \
+    *Synchronizing\ *\ groups* | \
+    \*\*\ Synchronizing\ *\ users* | \
+    --\ Updating\ user\ \'*\'?\(*\)* | \
+    --\ Updating\ group\ \'*\'* | \
+    --\ Creating\ user\ \'*\'?\(*\)* | \
+    --\ Creating\ group\ \'*\'*)
+      last_action="$line"
+      ;;
+    *)
+      echo "$line"
+      ;;
+  esac
 done