Hi Wojciech,
Thanks for the patch. Comments below.
On Fri, Jun 24, 2011 at 01:37:20AM +0100, Wojciech Meyer wrote:
> +(defun lines-from-string (string)
> + "Boiler plate for splitting STRING buffer to lines. Rewritten
> +thousands of times already"
> + (loop for i = 0 then (1+ j)
> + as j = (position #\Newline string :start i)
> + collect (subseq string i j)
> + while j))
Stumpwm already has a split-string function you could use for this.
To split on newlines you'd call it as follows:
(split-string str '(#\Newline))
> +(defun pair-split (entry separator)
> + "Split two ENTRY sections separated by SEPARATOR into pair"
> + (let ((lst (cl-ppcre:split separator entry)))
> + (cons (remove #\Newline (car lst)) (remove #\Newline (cadr lst)))))
This function is a bit confusing. The newline-removing behaviour
isn't described by the name or the docstring, and doesn't seem to be
required by at least two of the callers. Seems like you could just
call cl-ppcre:split directly for those callers and not bother with
this function.
> +(defun command-options (options &optional argument)
> + "Create command OPTIONS with optional ARGUMENTS. BUGS: Need to
> +insert space after each switch, should interleave space."
> + (format nil "~a ~a"
> + (if (listp options)
> + (apply #'concat options) options)
> + (or argument "") t))
You can do what you want with format and the ~{ directive here,
something like:
(format nil "~{~a~^ ~}" options)
I'm not really sure why this function takes two separate arguments
rather than just a simple list of options though.
Should the strings be shell escaped?
> +(defun pwsafe-command (password options &optional argument)
> + "Execute pwsafe command using master PASSWORD with OPTIONS and
> +additional ARGUMENT. Not safe."
> + (format nil "echo \"~a\" | pwsafe ~a" password (command-options options
> argument)))
Again, the password should probably be shell escaped, and I'm not sure
why options are argument are separate. It seems like this function
couild be merged with command-options since it's the only thing that
calls it. The resulting function could just take a password and a
list of options.
> +(defun with-xsel (command &optional options)
> + "Pipe COMMAND with OPTIONS to xsel."
> + (format nil "~a | xsel ~a" command (or options "")))
You could perhaps do this using clx. Stump already has a
set-x-selection function, it could be useful for it to have
set-x-clipboard too.
Otherwise, shell escaping again. It might also be clearer to make
options default to "" rather than using or.
> +(defun run-pwsafe-command (password options &optional argument)
> + (let ((output (run-shell-command (pwsafe-command password options
> argument) t)))
> + (when (cl-ppcre::scan "Passphrase is incorrect" output)
> + (throw 'error "Passphrase is incorrect"))
> + output))
Is this error on stdout? run-shell-command doesn't collect output
from stderr, so if it goes there this won't work without a 2>&1 in the
command.
> +(defun assoc-entries (entries)
> + "Create assoc from ENTRIES keyed by the name"
> + (mapcar (lambda (entry) (cons (pwsafe-entry-name entry) entry)) entries))
This seems to happen every time pwsafe-entries is called, so why not
do it as part of that rather than iterating over the list twice?
> +(defun pwsafe-password-to-clipboard (entry)
> + "Main function that will perform side action on ENTRY with all the
> +associated side effects like priting message and putting password into
> +xclipboard"
> + (let* ((pwsafe-entry (pwsafe-entry-name entry))
> + (output
> + (run-pwsafe-command (pwsafe-entry-password entry)
> + ;; FIXME: dirty hack, attempted to be nice
> + ;; and keep the options in list, however
> + ;; `pwsafe-command' is not interleaving
> + ;; them with space
> + '("-q " "-p " "--echo " "-E ") pwsafe-entry))
> + ;; FIXME: this one is a bit buggy, it should be really not
> + ;; calling pair-split it might not work in all cases
> + (entry-password (cdr (pair-split output "passphrase for.*: "))))
Not really sure what's going on here, if you give me an example of the
output I might be able to help with parsing it.
> + (set-x-selection entry-password)
> + (run-shell-command (with-xsel (format nil "echo \"~a\"" entry-password)
> "-ib") t)
> + (message
> + (format nil "Username: ~a (password copied to clipboard)"
> + (pwsafe-entry-user-name entry)))))
message takes a format string and args so there should be no need for
a separate call to format.
> +(defcommand pwsafe-menu (password) ((:password "Pwsafe password: "))
> + "Prompt for PASSWORD. Show menu with pwsafe database entries. Let
> +the user choose entry, put password to clipboard and notify user about
> +associated username"
> + (let* ((entries (pwsafe-entries password))
> + (entry (select-from-menu (current-screen)
> + (assoc-entries entries))))
select-from-menu is not currently exported, you'll need to either
export it or call it with stumpwm:: here (exporting it seems
reasonable).
> +(define-stumpwm-type :pwsafe-entry (input prompt)
> + "This is our type for prompting entry, and performing completion."
> + (or (argument-pop input)
This looks iffy, won't it return a string rather than the expected
pwsafe-entry?
Ben
_______________________________________________
Stumpwm-devel mailing list
Stumpwm-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/stumpwm-devel
