From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
To: Jani Nikula <jani@nikula.org>, notmuch@notmuchmail.org
Subject: Re: [PATCH 1/3] emacs: bind "s" to `notmuch-search' in notmuch-hello
	buffer
In-Reply-To: <874nvukort.fsf@nikula.org>
References: <1326828850-8519-1-git-send-email-dmitry.kurochkin@gmail.com>
	<874nvukort.fsf@nikula.org>
User-Agent: Notmuch/0.11+73~g8a5402e (http://notmuchmail.org) Emacs/23.3.1
	(x86_64-pc-linux-gnu)
Date: Wed, 18 Jan 2012 01:29:36 +0400
Message-ID: <87mx9mvwzj.fsf@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

[of course I sent this email privately, sorry for duplicates]

On Tue, 17 Jan 2012 23:22:30 +0200, Jani Nikula <jani@nikula.org> wrote:
> On Tue, 17 Jan 2012 23:34:08 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > Before the change, "s" in notmuch-hello buffer would jump to the
> > search box.  The patch changes the binding to `notmuch-search' which
> > is consistent with all other notmuch buffers.
> > ---
> >  emacs/notmuch-hello.el |   19 ++++++-------------
> >  1 files changed, 6 insertions(+), 13 deletions(-)
> > 
> > diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
> > index 02017ce..08fcd22 100644
> > --- a/emacs/notmuch-hello.el
> > +++ b/emacs/notmuch-hello.el
> > @@ -29,9 +29,6 @@
> >  (declare-function notmuch-search "notmuch" (query &optional oldest-first target-thread target-line continuation))
> >  (declare-function notmuch-poll "notmuch" ())
> >  
> > -(defvar notmuch-hello-search-bar-marker nil
> > -  "The position of the search bar within the notmuch-hello buffer.")
> > -
> >  (defcustom notmuch-recent-searches-max 10
> >    "The number of recent searches to store and display."
> >    :type 'integer
> > @@ -321,11 +318,6 @@ should be. Returns a cons cell `(tags-per-line width)'."
> >  	(widget-insert "\n"))
> >      found-target-pos))
> >  
> > -(defun notmuch-hello-goto-search ()
> > -  "Put point inside the `search' widget."
> > -  (interactive)
> > -  (goto-char notmuch-hello-search-bar-marker))
> 
> After this, what would the user have to do to bind some key to put the
> point in the search box? If someone wants to restore old behaviour for
> themselves.
> 

There is no way to do that.  From the previous discussions, I do not
remember anyone wanted the current "s" jumping behavior.  Let's wait for
some feedback and see if anyone wants that.  I would prefer to remove
this function and the global variable it uses.

> Also, it's perhaps out of scope for this patch, but it will become more
> evident now that notmuch-search does not respect
> notmuch-search-oldest-first when called without parameters like the new
> 's' keybinding does. This is the same in search view.
> 

I agree that this is an issue.  But it is outisde of the scope of this
patch series.

Regards,
  Dmitry

> 
> BR,
> Jani.
> 
> 
> > -
> >  (defimage notmuch-hello-logo ((:type png :file "notmuch-logo.png")))
> >  
> >  (defun notmuch-hello-search-continuation()
> > @@ -355,7 +347,7 @@ should be. Returns a cons cell `(tags-per-line width)'."
> >      (define-key map "G" 'notmuch-hello-poll-and-update)
> >      (define-key map (kbd "<C-tab>") 'widget-backward)
> >      (define-key map "m" 'notmuch-mua-new-mail)
> > -    (define-key map "s" 'notmuch-hello-goto-search)
> > +    (define-key map "s" 'notmuch-search)
> >      map)
> >    "Keymap for \"notmuch hello\" buffers.")
> >  (fset 'notmuch-hello-mode-map notmuch-hello-mode-map)
> > @@ -468,7 +460,8 @@ Complete list of currently available key bindings:
> >        (widget-insert " messages.\n"))
> >  
> >      (let ((found-target-pos nil)
> > -	  (final-target-pos nil))
> > +	  (final-target-pos nil)
> > +	  (search-bar-pos))
> >        (let* ((saved-alist
> >  	      ;; Filter out empty saved searches if required.
> >  	      (if notmuch-show-empty-saved-searches
> > @@ -500,7 +493,7 @@ Complete list of currently available key bindings:
> >  	    (indent-rigidly start (point) notmuch-hello-indent)))
> >  
> >  	(widget-insert "\nSearch: ")
> > -	(setq notmuch-hello-search-bar-marker (point-marker))
> > +	(setq search-bar-pos (point-marker))
> >  	(widget-create 'editable-field
> >  		       ;; Leave some space at the start and end of the
> >  		       ;; search boxes.
> > @@ -589,7 +582,7 @@ Complete list of currently available key bindings:
> >  	(when notmuch-saved-searches
> >  	  (widget-insert "Edit saved searches with the `edit' button.\n"))
> >  	(widget-insert "Hit RET or click on a saved search or tag name to view matching threads.\n")
> > -	(widget-insert "`=' refreshes this screen. `s' jumps to the search box. `q' to quit.\n")
> > +	(widget-insert "`=' refreshes this screen. `s' to search messages. `q' to quit.\n")
> >  	(let ((fill-column (- (window-width) notmuch-hello-indent)))
> >  	  (center-region start (point))))
> >  
> > @@ -601,7 +594,7 @@ Complete list of currently available key bindings:
> >  	  (widget-forward 1)))
> >  
> >        (unless (widget-at)
> > -	(notmuch-hello-goto-search))))
> > +	(goto-char search-bar-pos))))
> >  
> >    (run-hooks 'notmuch-hello-refresh-hook))
> >  
> > -- 
> > 1.7.8.3
> > 
> > _______________________________________________
> > notmuch mailing list
> > notmuch@notmuchmail.org
> > http://notmuchmail.org/mailman/listinfo/notmuch