[#476] Use getopt for argument parsing

README.md

@@ -111,11 +111,17 @@ Compile the project:
 These are the command-line arguments that can be provided to the
 `erlang_ls` escript:
 
-| Argument                | Description                                                                                     |
-|-------------------------|-------------------------------------------------------------------------------------------------|
-| --transport tcp / stdio | Specifies the transport the server will use for the connection with the client (default: `tcp`) |
-| --port PORT             | Used when the transport is `tcp` (default: `10000`)                                             |
-| --log-dir DIR           | Directory where logs will be written. When not provided no logs are generated.                  |
+``` shell
+Usage: Erlang LS [-v] [-t [<transport>]] [-p [<port>]] [-d [<log_dir>]]
+                 [-l [<log_level>]] [<port_old>]
+
+  -v, --version    Print the current version of Erlang LS
+  -t, --transport  Specifies the transport the server will use for the
+                   connection with the client. [default: tcp]
+  -p, --port       Used when the transport is tcp. [default: 10000]
+  -d, --log-dir    Directory where logs will be written. [default: <user_log>/erlang_ls/log]
+  -l, --log-level  The log level that should be used. [default: info]
+```
 
 ### Emacs Setup
 

elvis.config

@@ -47,6 +47,7 @@
                                                                    , {left , "=/="}
                                                                    ]}}
                       , {elvis_style, function_naming_convention, #{ignore => [els_client]}}
+                      , {elvis_style, no_debug_call, #{ignore => [erlang_ls]}}
                       ]
          , ignore => [els_dodger]
          }

src/erlang_ls.app.src

@@ -16,6 +16,7 @@
       , ephemeral
       , rebar3_format
       , tdiff
+      , getopt
       ]
     }
   , { env

src/erlang_ls.erl

@@ -14,6 +14,7 @@
 main(Args) ->
   %% Initialization
   application:load(lager),
+  application:load(getopt),
   application:load(?APP),
   ok = parse_args(Args),
   ok = lager_config(),
@@ -22,38 +23,100 @@ main(Args) ->
   lager:info("Started erlang_ls server", []),
   receive _ -> ok end.
 
+-spec print_version() -> ok.
+print_version() ->
+  {ok, Vsn} = application:get_key(?APP, vsn),
+  io:format("Version: ~s~n", [Vsn]),
+  ok.
+
 %%==============================================================================
 %% Argument parsing
 %%==============================================================================
 
 -spec parse_args([string()]) -> ok.
-parse_args([]) ->
-  ok;
-parse_args(["--transport", Name | Rest]) ->
+parse_args(Args) ->
+  case getopt:parse(opt_spec_list(), Args) of
+    {ok, {[version | _], _BadArgs}} ->
+      print_version(),
+      halt(1);
+    {ok, {ParsedArgs, _BadArgs}} ->
+      %% Handle backwards compatibility of clients that do not provide args but
+      %% only supply an integer. Assume it to be the port and remove the default
+      %% that we pick for the port.
+      ValidArgs = case lists:keyfind(port_old, 1, ParsedArgs) of
+                    false -> ParsedArgs;
+                    {port_old, _} -> lists:keydelete(port, 1, ParsedArgs)
+                  end,
+      set_args(ValidArgs);
+    {error, {invalid_option, _}} ->
+      getopt:usage(opt_spec_list(), "Erlang LS"),
+      halt(1)
+  end.
+
+-spec opt_spec_list() -> [getopt:option_spec()].
+opt_spec_list() ->
+  [ { version
+    , $v
+    , "version"
+    , undefined
+    , "Print the current version of Erlang LS"
+    }
+  , { transport
+    , $t
+    , "transport"
+    , {string, "tcp"}
+    , "Specifies the transport the server will use for "
+      "the connection with the client."
+    }
+  , { port
+    , $p
+    , "port"
+    , {integer, 10000}
+    , "Used when the transport is tcp."
+    }
+ ,  { log_dir
+    , $d
+    , "log-dir"
+    , {string, filename:basedir(user_log, "erlang_ls")}
+    , "Directory where logs will be written."
+    }
+ ,  { log_level
+    , $l
+    , "log-level"
+    , {string, ?DEFAULT_LOGGING_LEVEL}
+    , "The log level that should be used."
+    }
+ ,  { port_old
+    , undefined
+    , undefined
+    , integer
+    , "Port provided as integer for backwards compatibility reasons"
+    }
+  ].
+
+-spec set_args([] | [getopt:compound_option()]) -> ok.
+set_args([]) -> ok;
+set_args([version | Rest]) -> set_args(Rest);
+set_args([{Arg, Val} | Rest]) ->
+  set(Arg, Val),
+  set_args(Rest).
+
+-spec set(atom(), getopt:arg_value()) -> ok.
+set(transport, Name) ->
   Transport = case Name of
                 "tcp"   -> els_tcp;
                 "stdio" -> els_stdio
               end,
-  application:set_env(?APP, transport, Transport),
-  parse_args(Rest);
-parse_args(["--port", Port | Rest]) ->
-  application:set_env(?APP, port, list_to_integer(Port)),
-  parse_args(Rest);
-parse_args(["--log-dir", Dir | Rest]) ->
+  application:set_env(?APP, transport, Transport);
+set(port, Port) ->
+  application:set_env(?APP, port, Port);
+set(log_dir, Dir) ->
   application:set_env(?APP, logging_enabled, true),
-  application:set_env(?APP, log_dir, Dir),
-  parse_args(Rest);
-parse_args(["--log-level", Level | Rest]) ->
-  case Level of
-    "none" -> ok;
-    _      -> application:set_env(?APP, logging_enabled, true)
-  end,
-  application:set_env(?APP, log_level, Level),
-  parse_args(Rest);
-%% For backward compatibility with clients
-parse_args([Port | Rest]) ->
-  application:set_env(?APP, port, list_to_integer(Port)),
-  parse_args(Rest).
+  application:set_env(?APP, log_dir, Dir);
+set(log_level, Level) ->
+  application:set_env(?APP, log_level, Level);
+set(port_old, Port) ->
+  application:set_env(?APP, port, Port).
 
 %%==============================================================================
 %% Lager configuration
@@ -78,7 +141,7 @@ lager_config() ->
 -spec lager_handlers(string()) -> [any()].
 lager_handlers(LogRoot) ->
   LogFile      = filename:join([LogRoot, "server.log"]),
-  LoggingLevel = application:get_env(?APP, log_level, ?DEFAULT_LOGGING_LEVEL),
+  {ok, LoggingLevel} = application:get_env(?APP, log_level),
   ok           = filelib:ensure_dir(LogFile),
   [ { lager_file_backend
     , [ {file, LogFile}
@@ -89,8 +152,7 @@ lager_handlers(LogRoot) ->
 
 -spec log_root() -> string().
 log_root() ->
-  DefaultLogDir    = filename:basedir(user_log, "erlang_ls"),
-  LogDir           = application:get_env(?APP, log_dir, DefaultLogDir),
+  {ok, LogDir}     = application:get_env(?APP, log_dir),
   {ok, CurrentDir} = file:get_cwd(),
   Dirname          = filename:basename(CurrentDir),
   filename:join([LogDir, Dirname]).

test/erlang_ls_SUITE.erl

@@ -105,6 +105,10 @@ lager_config(_Config) ->
   ?assertEqual(false, application:get_env(lager, crash_log, undefined)),
 
   application:set_env(erlang_ls, logging_enabled, true),
+  application:set_env(erlang_ls, log_level, "info"),
+  application:set_env(erlang_ls,
+                      log_dir,
+                      filename:basedir(user_log, "erlang_ls")),
   erlang_ls:lager_config(),
   ?assertEqual(1, length(application:get_env(lager, handlers, []))),
   ?assertEqual("crash.log", application:get_env(lager, crash_log, undefined)),
@@ -117,6 +121,7 @@ log_handlers(_Config) ->
   meck:new(filelib, [unstick]),
   meck:expect(filelib, ensure_dir, fun(_) -> ok end),
 
+  application:set_env(erlang_ls, log_level, "info"),
   Handlers = erlang_ls:lager_handlers("/some/directory"),
   ExpectedHandlers = [ { lager_file_backend
                        , [ {file, "/some/directory/server.log"}