Add changes from sorin codereview

-    autoload functions aren't surrounded in an addition function { }
-    export cross platform notify function
-    control notify behavior with zstyles

modules/last_command/functions/time_to_human

@@ -6,21 +6,19 @@
 #
 
 # Converts a floating point time duration in seconds to a human readable string.
-function time_to_human {
-  seconds=$1
-  if (( seconds < 10 )); then
-    printf "%6.3fs" $seconds
-  elif (( seconds < 60 )); then
-    printf "%6.3fs" $seconds
-  elif (( seconds < (60*60) )); then
-    printf "%6.3fm" $(( seconds / 60 ))
-  elif (( seconds < (60*60*24) )); then
-    printf "%6.3fh" $(( seconds / (60*60) ))
-  elif (( seconds < (60*60*24*30) )); then
-    printf "%6.3fd" $(( seconds / (60*60*24) ))
-  elif (( seconds < (60*60*24*30*12) )); then
-    printf "%6.3fm" $(( seconds / (60*60*24*30) ))
-  else
-    printf "%6.3fy" $(( seconds / (60*60*24*30*12) ))
-  fi
-}
+seconds=$1
+if (( seconds < 10 )); then
+  printf "%6.3fs" $seconds
+elif (( seconds < 60 )); then
+  printf "%6.3fs" $seconds
+elif (( seconds < (60*60) )); then
+  printf "%6.3fm" $(( seconds / 60 ))
+elif (( seconds < (60*60*24) )); then
+  printf "%6.3fh" $(( seconds / (60*60) ))
+elif (( seconds < (60*60*24*30) )); then
+  printf "%6.3fd" $(( seconds / (60*60*24) ))
+elif (( seconds < (60*60*24*30*12) )); then
+  printf "%6.3fm" $(( seconds / (60*60*24*30) ))
+else
+  printf "%6.3fy" $(( seconds / (60*60*24*30*12) ))
+fi

modules/last_command/init.zsh

@@ -1,20 +1,21 @@
 #
-# Exports information about the last command.
+# Provides information about the last command.
 #
 # Authors:
 #   Alex Reece <awreece@gmail.com>
 #
 
-# Exported values.
+# Provided values.
 last_command=''
 last_command_status=0
 last_command_time=0.0
 
-# Not exported.
-last_command_start_time=invalid
-
 zmodload zsh/datetime  # For EPOCHREALTIME.
 
+# This value is used internally by this module and is not intended to be used
+# elsewhere.
+last_command_start_time='invalid'
+
 function last_command_precmd {
   exit_status=$?  # TODO(awreece) What happens if another precmd runs first?
 

modules/node/init.zsh

@@ -16,6 +16,8 @@ if (( ! $+commands[node] )); then
   return 1
 fi
 
+pmodload 'helper'
+
 # Load NPM completion.
 if (( $+commands[npm] )); then
   cache_file="${0:h}/cache.zsh"

modules/notify/README.md

@@ -3,10 +3,28 @@ Last Command
 
 Notifies if the command completes and terminal isn't the foreground window.
 
-Caveats
--------
-Currently only works on Mac OSX with `terminal-notifier`. At some point, I
-will add support for other notification mechanisms.
+Settings
+--------
+
+### Growl
+
+By default, `notify` will use the Notification Center on Mac OSX via
+`terminal-notifier` and will fall back to Growl if this is not available. To
+force the use of growl, add the following to *zpreztorc*.
+
+    zstyle ':prezto:module:notify' force-growl 'yes'
+
+### Auto-notify if window not focused
+
+This module can automatically notify if a (long running) command completes
+while the terminal window does not have focus. This will pop up a notification
+with information about the last command like this:
+
+![Last command notification](http://codearcana.com/images/zsh_theme_popup.png)
+
+To enable this feature, add the following to *zpreztorc*:
+
+    zstyle ':prezto:module:notify' auto-notify 'yes'
 
 Authors
 -------

modules/notify/functions/focused_window_id

@@ -4,23 +4,21 @@
 #   Alex Reece <awreece@gmail.com>
 #
 
-function focused_window_id {
-  case "$OSTYPE" in
-    (darwin*)
-      osascript -e 'tell application "System Events" to ¬' \
-                -e '  set focus_app_name to ¬' \
-                -e '    name of first application process ¬' \
-                -e '    whose frontmost is true' \
-                -e 'tell application focus_app_name to ¬' \
-                -e '  get id of front window'
-      ;;
-    (linux-gnu*)
-      # http://stackoverflow.com/a/8688624/447288 
-      xprop -root -f _NET_ACTIVE_WINDOW 0x ' $0' _NET_ACTIVE_WINDOW  \
-        | awk '{print $2}'
-      ;;
-    (*)
-      return 1
-      ;;
-  esac
-}
+case "$OSTYPE" in
+  (darwin*)
+    osascript -e 'tell application "System Events" to ¬' \
+              -e '  set focus_app_name to ¬' \
+              -e '    name of first application process ¬' \
+              -e '    whose frontmost is true' \
+              -e 'tell application focus_app_name to ¬' \
+              -e '  get id of front window'
+    ;;
+  (linux-gnu*)
+    # http://stackoverflow.com/a/8688624/447288
+    xprop -root -f _NET_ACTIVE_WINDOW 0x ' $0' _NET_ACTIVE_WINDOW  \
+      | awk '{print $2}'
+    ;;
+  (*)
+    return 1
+    ;;
+esac

modules/notify/functions/notify

@@ -0,0 +1,27 @@
+# Pops up a notification with the provided arg.
+#
+# Authors:
+#   Alex Reece <awreece@gmail.com>
+#
+
+# I actually want all the args as one string.
+message="$*"
+
+case "$OSTYPE" in
+  (darwin*)
+    if is-callable terminal-notifier && \
+       ! zstyle -t ':prezto:module:notify' force-growl; then
+      terminal-notifier >/dev/null -message $message
+    elif is-callable growlnotify; then
+      growlnotify --message $message
+    else
+      return 1
+    fi
+    ;;
+  (linux-gnu*)
+    notify-send $message
+    ;;
+  (*)
+    return 1
+    ;;
+esac

modules/notify/functions/notify_last_command

@@ -0,0 +1,40 @@
+# Sends a notification that the last command completed.
+#
+# Assumes $terminal_window_id is the id of the terminals window and that
+# last_comand module is loaded.
+#
+# Authors:
+#   Alex Reece <awreece@gmail.com>
+#
+
+message=$(printf "Command \"%s\" finished (%d) after %s." \
+    $last_command \
+    $last_command_status \
+    $(time_to_human $last_command_time))
+
+# We duplicate a lot of the functionality of notify, but it means we can
+# have this nifty callback to select the window that finished.
+
+# TODO(awreece) Add support for user defined callback.
+case "$OSTYPE" in
+  (darwin*)
+    if is-callable terminal-notifier && 
+       ! zstyle -t ':prezto:module:notify' force-growl; then
+      callback="osascript -e 'tell application \"Terminal\"' \
+                          -e 'activate' \
+                          -e 'set index of window id $terminal_window_id to 1' \
+                          -e 'end tell'"
+      terminal-notifier -message $message -execute $callback >/dev/null
+    elif is-callable growlnotify; then
+      growlnotify --message $message
+    else
+     return 1
+    fi
+    ;;
+  (linux-gnu*)
+    notify-send "Command finished" $message
+    ;;
+  (*)
+    return 1
+    ;;
+esac

modules/notify/functions/should_auto_notify

@@ -0,0 +1,35 @@
+# Check if it makes sense to load the notify module.
+#
+# Checks for requirements, that it is an xsession, etc.
+#
+# Authors:
+#   Alex Reece <awreece@gmail.com>
+#
+
+if ! zstyle -t ':prezto:module:notify' auto-notify; then
+  return 1
+fi
+
+case "$OSTYPE" in
+  (darwin*) 
+    # TODO(awreece) Disable if ssh with no windows?
+    is-callable terminal-notifier || is-callable growlnotify
+    ;;
+  (linux-gnu*)
+    # Disable if don't have X.
+    if [[ -z $XAUTHORITY ]]; then
+      return 1
+    fi
+
+    # We need both of these functions to operate.
+    if (( ! $+commands[notify-send] || ! $+commands[xprop] )); then
+      return 1
+    fi
+
+    return 0
+    ;;
+  (*)
+    # If we don't know, then just disable.
+    return 1
+    ;;
+esac

modules/notify/functions/should_load_notify_module

@@ -1,34 +0,0 @@
-# Check if it makes sense to load the notify module.
-#
-# Checks for requirements, that it is an xsession, etc.
-#
-# Authors:
-#   Alex Reece <awreece@gmail.com>
-#
-
-function should_load_notify_module {
-  case "$OSTYPE" in
-    (darwin*) 
-      # For now, always enable on mac osx. 
-      # TODO(awreece) Disable if ssh with no windows?
-      return 0
-      ;;
-    (linux-gnu*)
-      # Disable if don't have X.
-      if [[ -z $XAUTHORITY ]]; then
-        return 1
-      fi
-
-      # We need both of these functions to operate.
-      if (( ! $+commands[notify-send] || ! $+commands[xprop] )); then
-        return 1
-      fi
-
-      return 0
-      ;;
-    (*) 
-      # If we don't know, then just disable.
-      return 1
-      ;;
-  esac
-}

modules/notify/init.zsh

@@ -5,54 +5,21 @@
 #   Alex Reece <awreece@gmail.com>
 #
 
-pmodload 'last_command'
+pmodload 'helper'
 
-if ! should_load_notify_module; then
-  return 1
-fi
-
-# Initialize $terminal_window_id to the current active window.
-terminal_window_id=$(focused_window_id)
-
-# Returns true if the current window has focus.
-#
-# TODO(awreece) Add support for tabs.
-function window_is_focused {
-  [[ $(focused_window_id) == $terminal_window_id ]]
-}
-
-# Sends a notification that the last command terminated.
-# Warning: currently only implemented for mac.
-function last_command_notify {
-  message=$(printf "Command \"%s\" finished (%d) after %s." \
-                   $last_command \
-                   $last_command_status \
-                   $(time_to_human $last_command_time))
+if should_auto_notify; then
+  pmodload 'last_command'
 
-  # TODO(awreece) Add support for user defined callback.
-  case "$OSTYPE" in
-    (darwin*)
-      callback="osascript -e 'tell application \"Terminal\"' \
-                          -e 'activate' \
-                          -e 'set index of window id $terminal_window_id to 1' \
-                          -e 'end tell'"
-      terminal-notifier -message $message -execute $callback >/dev/null
-      ;;
-    (linux-gnu*)
-      notify-send "Command finished" $message
-      ;;
-    (*)
-      return 1
-      ;;
-  esac
-}
+  # Initialize $terminal_window_id to the current active window.
+  terminal_window_id=$(focused_window_id)
 
-function notify_precmd {
-  if ! window_is_focused; then
-    last_command_notify
-  fi
-}
+  function notify_precmd {
+    if [[ $(focused_window_id) != $terminal_window_id ]]; then
+      notify_last_command
+    fi
+  }
 
-autoload -Uz add-zsh-hook
+  autoload -Uz add-zsh-hook
 
-add-zsh-hook precmd notify_precmd
+  add-zsh-hook precmd notify_precmd
+fi