more friendly extra arguments (#32)

* more friendly extra arguments

* add unit test

README.md

@@ -88,14 +88,14 @@ end
 ```
 
 
-## Tweaking behaviour via listener extra arguments
+## Tweaking behaviour via extra arguments
 
-For each platform, you can pass extra arguments to the underlying listener process via the `listener_extra_args` option.
+For each platform, you can pass extra arguments to the underlying listener process.
+
+Each backend support different extra arguments, check backend module documentation for more information.
 
 Here is an example to get instant notifications on file changes for Mac OS X:
 
 ```elixir
-FileSystem.start_link(dirs: ["/path/to/some/files"], listener_extra_args: "--latency=0.0")
+FileSystem.start_link(dirs: ["/path/to/some/files"], latency: 0, watch_root: true)
 ```
-
-See the [fs source](https://github.com/synrc/fs/tree/master/c_src) for more details.

config/config.exs

-# This file is responsible for configuring your application
-# and its dependencies with the aid of the Mix.Config module.
 use Mix.Config
 
-# This configuration is loaded before any dependency and is restricted
-# to this project. If another project depends on this project, this
-# file won't be loaded nor affect the parent project. For this reason,
-# if you want to provide default values for your application for third-
-# party users, it should be done in your mix.exs file.
-
-# Sample configuration:
-#
-#     config :logger, :console,
-#       level: :info,
-#       format: "$date $time [$level] $metadata$message\n",
-#       metadata: [:user_id]
-
-# It is also possible to import configuration files, relative to this
-# directory. For example, you can emulate configuration per environment
-# by uncommenting the line below and defining dev.exs, test.exs and such.
-# Configuration from the imported file will override the ones defined
-# here (which is why it is important to import them last).
-#
-#     import_config "#{Mix.env}.exs"
+if :test == Mix.env do
+  config :logger, backends: []
+end

lib/file_system.ex

@@ -10,13 +10,13 @@ defmodule FileSystem do
       for `macos`, `:fs_inotify` for `linux` and `freebsd`,
       `:fs_windows` for `windows`
 
-    * `:listener_extra_args` (string, optional), extra args for
-      port backend.
-
     * `:name` (atom, optional), `name` can be used to subscribe as
       the same as pid when the `name` is given. The `name` should
       be the name of worker process.
 
+    * All rest options will treated as backend options. See backend
+      module documents for more details.
+
   ## Example
 
   Simple usage:
@@ -26,7 +26,7 @@ defmodule FileSystem do
 
   Get instant notifications on file changes for Mac OS X:
 
-      iex> FileSystem.start_link(dirs: ["/path/to/some/files"], listener_extra_args: "--latency=0.0")
+      iex> FileSystem.start_link(dirs: ["/path/to/some/files"], latency: 0)
 
   Named monitor with specified backend:
 

lib/file_system/backends/fs_inotify.ex

 require Logger
-alias FileSystem.Utils
 
 defmodule FileSystem.Backends.FSInotify do
   @moduledoc """
@@ -7,6 +6,10 @@ defmodule FileSystem.Backends.FSInotify do
   FileSysetm backend for linux and freebsd, a GenServer receive data from Port, parse event
   and send it to the worker process.
   Need `inotify-tools` installed to use this backend.
+
+  ## Backend Options
+
+    * `:recursive` (bool, default: true), monitor directories and their contents recursively
   """
 
   use GenServer
@@ -35,24 +38,56 @@ defmodule FileSystem.Backends.FSInotify do
     System.find_executable("inotifywait")
   end
 
+  def parse_options(options) do
+    case Keyword.pop(options, :dirs) do
+      {nil, _} ->
+        Logger.error "required argument `dirs` is missing"
+        {:error, :missing_dirs_argument}
+      {dirs, rest} ->
+        format = ["%w", "%e", "%f"] |> Enum.join(@sep_char) |> to_charlist
+        args = [
+          '-e', 'modify', '-e', 'close_write', '-e', 'moved_to', '-e', 'create',
+          '-e', 'delete', '-e', 'attrib', '--format', format, '--quiet', '-m', '-r'
+          | dirs |> Enum.map(&Path.absname/1) |> Enum.map(&to_charlist/1)
+        ]
+        parse_options(rest, args)
+    end
+  end
+
+  defp parse_options([], result), do: {:ok, result}
+  defp parse_options([{:recursive, true} | t], result) do
+    parse_options(t, result)
+  end
+  defp parse_options([{:recursive, false} | t], result) do
+    parse_options(t, result -- ['-r'])
+  end
+  defp parse_options([{:recursive, value} | t], result) do
+    Logger.error "unknown value `#{inspect value}` for recursive, ignore"
+    parse_options(t, result)
+  end
+  defp parse_options([h | t], result) do
+    Logger.error "unknown option `#{inspect h}`, ignore"
+    parse_options(t, result)
+  end
+
   def start_link(args) do
     GenServer.start_link(__MODULE__, args, [])
   end
 
   def init(args) do
-    port_path = Utils.format_path(args[:dirs])
-    format = ["%w", "%e", "%f"] |> Enum.join(@sep_char) |> to_charlist
-    port_args = [
-      '-e', 'modify', '-e', 'close_write', '-e', 'moved_to', '-e', 'create',
-      '-e', 'delete', '-e', 'attrib', '--format', format, '--quiet', '-m', '-r'
-    ] ++ Utils.format_args(args[:listener_extra_args]) ++ port_path
-    port = Port.open(
-      {:spawn_executable, to_charlist(find_executable())},
-      [:stream, :exit_status, {:line, 16384}, {:args, port_args}, {:cd, System.tmp_dir!()}]
-    )
-    Process.link(port)
-    Process.flag(:trap_exit, true)
-    {:ok, %{port: port, worker_pid: args[:worker_pid]}}
+    {worker_pid, rest} = Keyword.pop(args, :worker_pid)
+    case parse_options(rest) do
+      {:ok, port_args} ->
+        port = Port.open(
+          {:spawn_executable, to_charlist(find_executable())},
+          [:stream, :exit_status, {:line, 16384}, {:args, port_args}, {:cd, System.tmp_dir!()}]
+        )
+        Process.link(port)
+        Process.flag(:trap_exit, true)
+        {:ok, %{port: port, worker_pid: worker_pid}}
+      {:error, _} ->
+        :ignore
+    end
   end
 
   def handle_info({port, {:data, {:eol, line}}}, %{port: port}=state) do

lib/file_system/backends/fs_mac.ex

 require Logger
-alias FileSystem.Utils
 
 defmodule FileSystem.Backends.FSMac do
   @moduledoc """
   This file is a fork from https://github.com/synrc/fs.
   FileSysetm backend for macos, a GenServer receive data from Port, parse event
   and send it to the worker process.
-  will compile executable the buildin executable file when file the first time it is used.
+  Will compile executable the buildin executable file when file the first time it is used.
+
+  ## Backend Options
+
+    * `:latency` (float, default: 0.5), latency period.
+
+    * `:no_defer` (bool, default: false), enable no-defer latency modifier.
+      Works with latency parameter, Also check apple `FSEvent` api documents
+      https://developer.apple.com/documentation/coreservices/kfseventstreamcreateflagnodefer
+
+    * `:watch_root` (bool, default: false), watch for when the root path has changed.
+      Set the flag `true` to monitor events when watching `/tmp/fs/dir` and run
+      `mv /tmp/fs /tmp/fx`. Also check apple `FSEvent` api documents
+      https://developer.apple.com/documentation/coreservices/kfseventstreamcreateflagwatchroot
+
+    * recursive is enabled by default, no option to disable it for now.
   """
 
   use GenServer
@@ -44,21 +58,68 @@ defmodule FileSystem.Backends.FSMac do
     (:code.priv_dir(:file_system) ++ '/mac_listener') |> to_string
   end
 
+  def parse_options(options) do
+    case Keyword.pop(options, :dirs) do
+      {nil, _} ->
+        Logger.error "required argument `dirs` is missing"
+        {:error, :missing_dirs_argument}
+      {dirs, rest} ->
+        args = ['-F' | dirs |> Enum.map(&Path.absname/1) |> Enum.map(&to_charlist/1)]
+        parse_options(rest, args)
+    end
+  end
+
+  defp parse_options([], result), do: {:ok, result}
+  defp parse_options([{:latency, latency} | t], result) do
+    result =
+      if is_float(latency) or is_integer(latency) do
+        ['--latency=#{latency / 1}' | result]
+      else
+        Logger.error "latency should be integer or float, got `#{inspect latency}, ignore"
+        result
+      end
+    parse_options(t, result)
+  end
+  defp parse_options([{:no_defer, true} | t], result) do
+    parse_options(t, ['--no-defer' | result])
+  end
+  defp parse_options([{:no_defer, false} | t], result) do
+    parse_options(t, result)
+  end
+  defp parse_options([{:no_defer, value} | t], result) do
+    Logger.error "unknown value `#{inspect value}` for no_defer, ignore"
+    parse_options(t, result)
+  end
+  defp parse_options([{:with_root, true} | t], result) do
+    parse_options(t, ['--with-root' | result])
+  end
+  defp parse_options([{:with_root, value} | t], result) do
+    Logger.error "unknown value `#{inspect value}` for with_root, ignore"
+    parse_options(t, result)
+  end
+  defp parse_options([h | t], result) do
+    Logger.error "unknown option `#{inspect h}`, ignore"
+    parse_options(t, result)
+  end
 
   def start_link(args) do
     GenServer.start_link(__MODULE__, args, [])
   end
 
   def init(args) do
-    port_path = Utils.format_path(args[:dirs])
-    port_args = Utils.format_args(args[:listener_extra_args]) ++ ['-F' | port_path]
-    port = Port.open(
-      {:spawn_executable, to_charlist(find_executable())},
-      [:stream, :exit_status, {:line, 16384}, {:args, port_args}, {:cd, System.tmp_dir!()}]
-    )
-    Process.link(port)
-    Process.flag(:trap_exit, true)
-    {:ok, %{port: port, worker_pid: args[:worker_pid]}}
+    {worker_pid, rest} = Keyword.pop(args, :worker_pid)
+    case parse_options(rest) do
+      {:ok, port_args} ->
+        port = Port.open(
+          {:spawn_executable, to_charlist(find_executable())},
+          [:stream, :exit_status, {:line, 16384}, {:args, port_args}, {:cd, System.tmp_dir!()}]
+        )
+        Process.link(port)
+        Process.flag(:trap_exit, true)
+        {:ok, %{port: port, worker_pid: worker_pid}}
+      {:error, _} ->
+        :ignore
+    end
   end
 
   def handle_info({port, {:data, {:eol, line}}}, %{port: port}=state) do

lib/file_system/backends/fs_windows.ex

 require Logger
-alias FileSystem.Utils
 
 defmodule FileSystem.Backends.FSWindows do
   @moduledoc """
@@ -7,6 +6,10 @@ defmodule FileSystem.Backends.FSWindows do
   FileSysetm backend for windows, a GenServer receive data from Port, parse event
   and send it to the worker process.
   Need binary executable file packaged in to use this backend.
+
+  ## Backend Options
+
+    * `:recursive` (bool, default: true), monitor directories and their contents recursively
   """
 
   use GenServer
@@ -35,23 +38,55 @@ defmodule FileSystem.Backends.FSWindows do
     (:code.priv_dir(:file_system) ++ '/inotifywait.exe') |> to_string
   end
 
+  def parse_options(options) do
+    case Keyword.pop(options, :dirs) do
+      {nil, _} ->
+        Logger.error "required argument `dirs` is missing"
+        {:error, :missing_dirs_argument}
+      {dirs, rest} ->
+        format = ["%w", "%e", "%f"] |> Enum.join(@sep_char) |> to_charlist
+        args = [
+          '--format', format, '--quiet', '-m', '-r'
+          | dirs |> Enum.map(&Path.absname/1) |> Enum.map(&to_charlist/1)
+        ]
+        parse_options(rest, args)
+    end
+  end
+
+  defp parse_options([], result), do: {:ok, result}
+  defp parse_options([{:recursive, true} | t], result) do
+    parse_options(t, result)
+  end
+  defp parse_options([{:recursive, false} | t], result) do
+    parse_options(t, result -- ['-r'])
+  end
+  defp parse_options([{:recursive, value} | t], result) do
+    Logger.error "unknown value `#{inspect value}` for recursive, ignore"
+    parse_options(t, result)
+  end
+  defp parse_options([h | t], result) do
+    Logger.error "unknown option `#{inspect h}`, ignore"
+    parse_options(t, result)
+  end
+
   def start_link(args) do
     GenServer.start_link(__MODULE__, args, [])
   end
 
   def init(args) do
-    port_path = Utils.format_path(args[:dirs])
-    format = ["%w", "%e", "%f"] |> Enum.join(@sep_char) |> to_charlist
-    port_args = Utils.format_args(args[:listener_extra_args]) ++ [
-      '--format', format, '--quiet', '-m', '-r' | port_path
-    ]
-    port = Port.open(
-      {:spawn_executable, to_charlist(find_executable())},
-      [:stream, :exit_status, {:line, 16384}, {:args, port_args}, {:cd, System.tmp_dir!()}]
-    )
-    Process.link(port)
-    Process.flag(:trap_exit, true)
-    {:ok, %{port: port, worker_pid: args[:worker_pid]}}
+    {worker_pid, rest} = Keyword.pop(args, :worker_pid)
+    case parse_options(rest) do
+      {:ok, port_args} ->
+        port = Port.open(
+          {:spawn_executable, to_charlist(find_executable())},
+          [:stream, :exit_status, {:line, 16384}, {:args, port_args}, {:cd, System.tmp_dir!()}]
+        )
+        Process.link(port)
+        Process.flag(:trap_exit, true)
+        {:ok, %{port: port, worker_pid: worker_pid}}
+      {:error, _} ->
+        :ignore
+    end
   end
 
   def handle_info({port, {:data, {:eol, line}}}, %{port: port}=state) do

lib/file_system/utils.ex

-defmodule FileSystem.Utils do
-  @moduledoc false
-
-  @doc false
-  @spec format_path(String.t() | [String.t()]) :: [charlist()]
-  def format_path(path) when is_list(path) do
-    for i <- path do
-      i |> Path.absname |> to_charlist
-    end
-  end
-  def format_path(path) do
-    [path] |> format_path
-  end
-
-  @doc false
-  @spec format_args(nil | String.t() | [String.t()]) :: [charlist()]
-  def format_args(nil), do: []
-  def format_args(str) when is_binary(str) do
-    str |> String.split |> format_args
-  end
-  def format_args(list) when is_list(list) do
-    list |> Enum.map(&to_charlist/1)
-  end
-
-end

lib/file_system/worker.ex

@@ -8,18 +8,19 @@ defmodule FileSystem.Worker do
 
   @doc false
   def start_link(args) do
-    {args, opts} = Keyword.split(args, [:backend, :dirs, :listener_extra_args])
+    {opts, args} = Keyword.split(args, [:name])
     GenServer.start_link(__MODULE__, args, opts)
   end
 
   @doc false
   def init(args) do
-    case FileSystem.Backend.backend(args[:backend]) do
-      {:ok, backend} ->
-        {:ok, backend_pid} = backend.start_link([{:worker_pid, self()} | Keyword.drop(args, [:backend])])
-        {:ok, %{backend_pid: backend_pid, subscribers: %{}}}
-      {:error, _reason} ->
-        :ignore
+    {backend, rest} = Keyword.pop(args, :backend)
+    with {:ok, backend} <- FileSystem.Backend.backend(backend),
+         {:ok, backend_pid} <- backend.start_link([{:worker_pid, self()} | rest])
+    do
+      {:ok, %{backend_pid: backend_pid, subscribers: %{}}}
+    else
+      _ -> :ignore
     end
   end
 

test/backends/fs_inotify_test.exs

@@ -2,47 +2,75 @@ defmodule FileSystem.Backends.FSInotifyTest do
   use ExUnit.Case, async: true
   import FileSystem.Backends.FSInotify
 
-  test "dir write close" do
-    assert {"/one/two/file", [:modified, :closed]} ==
-      ~w|/one/two/ CLOSE_WRITE,CLOSE file| |> to_port_line |> parse_line
-  end
+  describe "options parse test" do
+    test "without :dirs" do
+      assert {:error, _} = parse_options([])
+      assert {:error, _} = parse_options([recursive: 1])
+    end
 
-  test "dir create" do
-    assert {"/one/two/file", [:created]} ==
-      ~w|/one/two/ CREATE file| |> to_port_line |> parse_line
-  end
+    test "supported options" do
+      assert {:ok, ['-e', 'modify', '-e', 'close_write', '-e', 'moved_to', '-e', 'create', '-e',
+                    'delete', '-e', 'attrib', '--format', [37, 119, 1, 37, 101, 1, 37, 102],
+                    '--quiet', '-m', '-r', '/tmp']} ==
+        parse_options(dirs: ["/tmp"], recursive: true)
 
-  test "dir moved to" do
-    assert {"/one/two/file", [:renamed]} ==
-      ~w|/one/two/ MOVED_TO file| |> to_port_line |> parse_line
-  end
+      assert {:ok, ['-e', 'modify', '-e', 'close_write', '-e', 'moved_to', '-e', 'create', '-e',
+                    'delete', '-e', 'attrib', '--format', [37, 119, 1, 37, 101, 1, 37, 102],
+                    '--quiet', '-m', '/tmp']} ==
+        parse_options(dirs: ["/tmp"], recursive: false)
+    end
 
-  test "dir is_dir create" do
-    assert {"/one/two/dir", [:created, :isdir]} ==
-      ~w|/one/two/ CREATE,ISDIR dir| |> to_port_line |> parse_line
+    test "ignore unsupported options" do
+      assert {:ok, ['-e', 'modify', '-e', 'close_write', '-e', 'moved_to', '-e', 'create', '-e',
+                    'delete', '-e', 'attrib', '--format', [37, 119, 1, 37, 101, 1, 37, 102],
+                    '--quiet', '-m', '/tmp']} ==
+        parse_options(dirs: ["/tmp"], recursive: false, unsuppported: :options)
+    end
   end
 
-  test "file write close" do
-    assert {"/one/two/file", [:modified, :closed]} ==
-      ~w|/one/two/file CLOSE_WRITE,CLOSE| |> to_port_line |> parse_line
-  end
+  describe "port line parse test" do
+    defp to_port_line(list), do: list |> Enum.join(<<1>>) |> to_charlist
 
-  test "file delete_self" do
-    assert {"/one/two/file", [:undefined]} ==
-      ~w|/one/two/file DELETE_SELF| |> to_port_line |> parse_line
-  end
+    test "dir write close" do
+      assert {"/one/two/file", [:modified, :closed]} ==
+        ~w|/one/two/ CLOSE_WRITE,CLOSE file| |> to_port_line |> parse_line
+    end
 
-  test "whitespace in path" do
-    assert {"/one two/file", [:modified, :closed]} ==
-      ["/one two", "CLOSE_WRITE,CLOSE", "file"] |> to_port_line |> parse_line
+    test "dir create" do
+      assert {"/one/two/file", [:created]} ==
+        ~w|/one/two/ CREATE file| |> to_port_line |> parse_line
+    end
 
-    assert {"/one/two/file 1", [:modified, :closed]} ==
-      ["/one/two", "CLOSE_WRITE,CLOSE", "file 1"] |> to_port_line |> parse_line
+    test "dir moved to" do
+      assert {"/one/two/file", [:renamed]} ==
+        ~w|/one/two/ MOVED_TO file| |> to_port_line |> parse_line
+    end
 
-    assert {"/one two/file 1", [:modified, :closed]} ==
-      ["/one two", "CLOSE_WRITE,CLOSE", "file 1"] |> to_port_line |> parse_line
-  end
+    test "dir is_dir create" do
+      assert {"/one/two/dir", [:created, :isdir]} ==
+        ~w|/one/two/ CREATE,ISDIR dir| |> to_port_line |> parse_line
+    end
 
-  defp to_port_line(list), do: list |> Enum.join(<<1>>) |> to_charlist
+    test "file write close" do
+      assert {"/one/two/file", [:modified, :closed]} ==
+        ~w|/one/two/file CLOSE_WRITE,CLOSE| |> to_port_line |> parse_line
+    end
+
+    test "file delete_self" do
+      assert {"/one/two/file", [:undefined]} ==
+        ~w|/one/two/file DELETE_SELF| |> to_port_line |> parse_line
+    end
+
+    test "whitespace in path" do
+      assert {"/one two/file", [:modified, :closed]} ==
+        ["/one two", "CLOSE_WRITE,CLOSE", "file"] |> to_port_line |> parse_line
+
+      assert {"/one/two/file 1", [:modified, :closed]} ==
+        ["/one/two", "CLOSE_WRITE,CLOSE", "file 1"] |> to_port_line |> parse_line
+
+      assert {"/one two/file 1", [:modified, :closed]} ==
+        ["/one two", "CLOSE_WRITE,CLOSE", "file 1"] |> to_port_line |> parse_line
+    end
+  end
 
 end

test/backends/fs_mac_test.exs

@@ -2,13 +2,35 @@ defmodule FileSystem.Backends.FSMacTest do
   use ExUnit.Case, async: true
   import FileSystem.Backends.FSMac
 
-  test "file modified" do
-    assert {"/one/two/file", [:inodemetamod, :modified]} ==
-      parse_line('37425557\t0x00011400=[inodemetamod,modified]\t/one/two/file')
+  describe "options parse test" do
+    test "without :dirs" do
+      assert {:error, _} = parse_options([])
+      assert {:error, _} = parse_options([latency: 1])
+    end
+
+    test "supported options" do
+      assert {:ok, ['--with-root', '--no-defer', '--latency=0.0', '-F', '/tmp']} ==
+        parse_options(dirs: ["/tmp"], latency: 0, no_defer: true, with_root: true)
+
+      assert {:ok, ['--no-defer', '--latency=1.1', '-F', '/tmp1', '/tmp2']} ==
+        parse_options(dirs: ["/tmp1", "/tmp2"], latency: 1.1, no_defer: true)
+    end
+
+    test "ignore unsupported options" do
+      assert {:ok, ['--latency=0.0', '-F', '/tmp']} ==
+        parse_options(dirs: ["/tmp"], latency: 0, unsuppported: :options)
+    end
   end
 
-  test "whitespace in path" do
-    assert {"/one two/file", [:inodemetamod, :modified]} ==
-      parse_line('37425557\t0x00011400=[inodemetamod,modified]\t/one two/file')
+  describe "port line parse test" do
+    test "file modified" do
+      assert {"/one/two/file", [:inodemetamod, :modified]} ==
+        parse_line('37425557\t0x00011400=[inodemetamod,modified]\t/one/two/file')
+    end
+
+    test "whitespace in path" do
+      assert {"/one two/file", [:inodemetamod, :modified]} ==
+        parse_line('37425557\t0x00011400=[inodemetamod,modified]\t/one two/file')
+    end
   end
 end