Re: [STUMP] [PATCH] Add command-line options support (also a request for commit access)

[Ben Spencer]

On Sat, Mar 05, 2011 at 08:18:15PM -0600, Krzysztof Drewniak wrote:
> This patch adds and documents support for command-line options. This in
> preparation for a --replace option (which I have not implemented). Right
> now, there are three options --replace, --help and --version . --help
> and --version do what you would expect. Also, the info documentation has
> been updated with the new option as well as the docs for my logout
> patch.

Hi,

A few comments on the patches themselves:

git reports whitespace errors in several of these, it would be helpful
if you could fix these before submitting.

Although it's not consistent everywhere, the stumpwm source generally
uses spaces rather than tabs for indentation.  If you're using emacs
you can put (setq indent-tabs-mode nil) into a 'lisp-mode-hook or
similar to do this automatically.

Patches 2 and 3 appear to contain fixes for issues with patch 1.  It
would make life easier for reviewers if you merged this sort of thing
into one final patch.

Patch 3 doesn't apply for me: http://paste.lisp.org/display/120262


As for the code:

(argv) appears to be a clisp specific function.  We generally put
implementation-specific stuff in wrappers.lisp, ideally with
implementations for clisp, sbcl and ccl at least (though in this case
if that proves difficult a placeholder that just returns an empty
list would probably be ok).

I don't think options really requires a class, a simple list (possibly
an alist if we need option values in future) would suffice and would
avoid having to list the available options twice.

There doesn't seem to be much point committing a placeholder for
(replace-wm) until it's actually implemented.


Ben