[Date Prev][Date Next][Thread Prev][Thread Next][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
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)".
CA45 09B5 5351 7C11 A9D1 7286 0036 9E45 1595 8BC0