[Date Prev][Date Next][Thread Prev][][Date Index][Thread Index]

Re: A git-ish patch for w3m-db-history



On 2019-01-30 19:50, Filipp Gunbin wrote:
> A few comments:

Thanks for the feedback. Did you try the code to verify that it operates?

> > +; ARGS is not used. It is necessary in order to >/dev/null unnecessary
>
> Usually full-line comments start with two semicolons.
>
> >  (defun w3m-db-history (&optional start size)
> [..]
> > +  (interactive)
> > +  (cond
> > +   ((or executing-kbd-macro noninteractive)
> > +    (when (not start) (setq start 0))
> > +    (when (not size)  (setq size w3m-db-history-display-size)))
> > +   (t ; called interactively; possibly indirectly
> > +    (setq start (read-number "start: " (or w3m-db-history-display-size 0)))
> > +    (setq size (read-number "size: " (or size 0)))))
>
> It's easier to read code when you have normal interactive form: that is,
> which either takes string arg-descriptor, or a list which may contain
> calls like read-something.  Reading anywhere outside this (interactive
> (list ...)) looks awkward.

It may look awkward because its an awkward situation, or because I'm not
a certified expert elisp programmer, so I'll welcome any suggestions to
address all the constraints of the circumstances:

1) Handle a truly non-interactive call.

2) Handle a kind-of non-interactive call, like when it's called by
   function w3m-history. This is expected to be the most common
   actual-use case, because we have a default keybinding for function
   w3m-history and we are already setting very many keybindings.

3) Handle a direct interactive call.

4) Perform input validation on the arguments to ensure they are
   integers.

If you have a chance, please let me know how the result looks and
performs. The prompts for the arguments probably need to be more
descriptive; maybe "Start at what history entry #?", and "How many
entries per page? (0 for all on one page)".

-- 
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0