[emacs-w3m:13095] Re: A git-ish patch for w3m-db-history

Boruch Baum <[email protected]>
Newsgroups gmane.emacs.w3m
Message-ID <[email protected]>
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.