From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.ciao.gmane.io!not-for-mail From: "Basil L. Contovounesios" Newsgroups: gmane.emacs.bugs Subject: bug#40863: [PATCH] Improve the display-time-world UI Date: Mon, 27 Apr 2020 23:58:31 +0100 Message-ID: <874kt4y1rc.fsf@tcd.ie> References: Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="ciao.gmane.io:159.69.161.202"; logging-data="38451"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Cc: 40863@debbugs.gnu.org To: Stefan Kangas Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Tue Apr 28 01:00:21 2020 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1jTCjd-0009wB-4k for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 28 Apr 2020 01:00:21 +0200 Original-Received: from localhost ([::1]:35926 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jTCjb-0005OQ-RH for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 27 Apr 2020 19:00:19 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:45124) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jTCjO-0005OI-NK for bug-gnu-emacs@gnu.org; Mon, 27 Apr 2020 19:00:07 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.90_1) (envelope-from ) id 1jTCjN-0004yE-Ka for bug-gnu-emacs@gnu.org; Mon, 27 Apr 2020 19:00:06 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:55073) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jTCjK-0004ea-0m for bug-gnu-emacs@gnu.org; Mon, 27 Apr 2020 19:00:05 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1jTCjJ-0005is-Vi for bug-gnu-emacs@gnu.org; Mon, 27 Apr 2020 19:00:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: "Basil L. Contovounesios" Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 27 Apr 2020 23:00:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 40863 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 40863-submit@debbugs.gnu.org id=B40863.158802836021922 (code B ref 40863); Mon, 27 Apr 2020 23:00:01 +0000 Original-Received: (at 40863) by debbugs.gnu.org; 27 Apr 2020 22:59:20 +0000 Original-Received: from localhost ([127.0.0.1]:38386 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1jTCie-0005hW-07 for submit@debbugs.gnu.org; Mon, 27 Apr 2020 18:59:20 -0400 Original-Received: from mail-wm1-f66.google.com ([209.85.128.66]:34641) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1jTCib-0005hF-HX for 40863@debbugs.gnu.org; Mon, 27 Apr 2020 18:59:18 -0400 Original-Received: by mail-wm1-f66.google.com with SMTP id v4so921304wme.1 for <40863@debbugs.gnu.org>; Mon, 27 Apr 2020 15:59:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tcd-ie.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=N/mAw6NVf533SBypI4jbF4mpQn9jqlj5FbcRa06wFpw=; b=OYcWn7wGN4vMgz0NQMa2MKHMUQ3cP+G2YTX4t1Sr2R0aDQjzPB3P7IjehIXnTf8SYO Xx/hsFDbe59XetMM4mc3DhbqfHzFxqdMc3Ne72g2VIv1+J0Ly7L+7K+U0YUfCAGSQila y07K10enzn5O4dTFPoXfAZmRahp9wrniTyv/d9GqfZe8dDIRHL/XRft34izI3bIFKhWx qwe4ER2o9dab875M9o3W+E28PwnEKZbv6I+LHWsdx4Q62LsgLs1OaZ0YCfyWms16Ds1C XYOEZr63obAwotZF8/X9QR2L5MXWCmNh6S3GMkYOGvAC6q4Wzq2GYW+7UVSpiGe5S1f0 2AkA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version; bh=N/mAw6NVf533SBypI4jbF4mpQn9jqlj5FbcRa06wFpw=; b=gvpH+e/IT1OJ3+7n8wOmODooPP7USDVsB/8bdNvXqBlUeGlBKlvUA2/Td4hWL4sxEb i41Fd1SkYNnLcQVhYVU36UCXvQ9CJYyPszyhEKIxxalDC74EYVRJs+lF2h57iwhpDc1o 4dNPL3vaUmcOG0EdtQrhxZYcxaxktj9OsrONMNxuAMdIHT/EtHEHsLkJFWhv2kZIKxCC +PX4bJJjdnCXjLZsXe7fKgpYrJOrZHC168XjGxybW0QaexuSW9jyphGZD7gNxbwl4wJH wMLVzQk+f29+NlN5ngHAKsfgohgj6qIi9E8nrN4BlzMMftGLCsL1Sn08L+Sh5Cix6P7O cuKg== X-Gm-Message-State: AGi0PubbEr+X3HiJV/DY69dcJ6aIXJh4Sy6MGTnoQkjqCeaExiRH1vkV lARgKMER9aqQ40rw+defteXyv0HnQug= X-Google-Smtp-Source: APiQypIE2gj0kqEALkbiYh0MNfbMdrDYloBEw1mPgWOq9X2Af7W7pOYnDgi+f8ltrPaz7Zh8GTiweA== X-Received: by 2002:a1c:80c3:: with SMTP id b186mr1186944wmd.117.1588028321483; Mon, 27 Apr 2020 15:58:41 -0700 (PDT) Original-Received: from localhost ([2a02:8084:20e2:c380:92bd:1bfd:38fc:fae2]) by smtp.gmail.com with ESMTPSA id s9sm24325085wrg.27.2020.04.27.15.58.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 27 Apr 2020 15:58:40 -0700 (PDT) In-Reply-To: (Stefan Kangas's message of "Sun, 26 Apr 2020 10:56:03 +0200") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-Received-From: 209.51.188.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:179188 Archived-At: Stefan Kangas writes: > I have made some improvements to the display-time-world UI. I divided > them up into four patches to ease review and merging of the individual > features. Please let me know what you think. Thanks for working on this, see my comments below. > (Of course I can squash the patches before pushing if that is preferable.) > > Patch 4 adds an alias 'world-clock'. Ideally, I would like to rename > the somewhat obscurely named 'display-world-time' to 'world-clock' and > make the old names into obsolete aliases. It would be good to hear > any opinions on that too. No strong feelings either way here. > I'm also not sure if any of this should go into NEWS, so I didn't do > that work for now. Let me know if that's desirable. A new entry-point command alias intended as a replacement for the current command should definitely be announced. Not sure about the rest. > Best regards, > Stefan Kangas > > From 3d8c091a03c25826755b3735eca9da4b342826ba Mon Sep 17 00:00:00 2001 > From: Stefan Kangas > Date: Sun, 26 Apr 2020 09:38:03 +0200 > Subject: [PATCH 1/4] Kill display-time-world buffer on exit > > * lisp/time.el (display-time-world-mode-map): New defvar. > (display-time-world-exit): New defun to kill buffer on exit. > (display-time-world): Doc fix. > --- > lisp/time.el | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/lisp/time.el b/lisp/time.el > index 44fd1a7e33..a5e5b9b4a7 100644 > --- a/lisp/time.el > +++ b/lisp/time.el > @@ -509,6 +509,13 @@ display-time-mode > 'display-time-event-handler))) > > > +(defvar display-time-world-mode-map > + (let ((map (make-sparse-keymap))) > + (define-key map "g" nil) Rather than remove the revert-buffer binding inherited from special-mode-map, wouldn't it be better to implement a revert-buffer-function that updates the world clocks displayed? > + (define-key map "q" 'display-time-world-exit) [...] > +(defun display-time-world-exit () > + "Quit the world time buffer." > + (interactive) > + (quit-window t)) I don't think this is a change for the better. The existing binding of quit-window, inherited by and consistent across many special-mode derivatives, already takes a prefix argument for killing the corresponding buffer if a user prefers that to burying. The proposed change would make killing the only option by default. > ;;;###autoload > (defun display-time-world () > "Enable updating display of times in various time zones. > +\\ > `display-time-world-list' specifies the zones. > -To turn off the world time display, go to that window and type `q'." > +To turn off the world time display, go to that window and type `\\[display-time-world-exit]'." If you agree with me then this can be changed to \\[quit-window]. [...] > @@ -540,7 +544,10 @@ display-time-world-display > (setq max-width width)))) > (setq fmt (concat "%-" (int-to-string max-width) "s %s\n")) > (dolist (timedata (nreverse result)) > - (insert (format fmt (car timedata) (cdr timedata)))) > + (insert (format fmt > + (propertize (car timedata) > + 'face 'display-time-world-label) > + (cdr timedata)))) Just curious: when do we use 'face' and when 'font-lock-face'? > (delete-char -1)) > (goto-char (point-min))) > > -- > 2.26.2 > > > From 0f2ca954e03b857554071ae48a98d44ba7030d69 Mon Sep 17 00:00:00 2001 > From: Stefan Kangas > Date: Sun, 26 Apr 2020 10:44:00 +0200 > Subject: [PATCH 3/4] Move point to new buffer for display-time-world > > * lisp/time.el (display-time-world): Move point to created buffer. > --- > lisp/time.el | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/lisp/time.el b/lisp/time.el > index 44885f74d1..84c0071b7d 100644 > --- a/lisp/time.el > +++ b/lisp/time.el > @@ -566,11 +566,11 @@ display-time-world > (when (and display-time-world-timer-enable > (not (get-buffer display-time-world-buffer-name))) > (run-at-time t display-time-world-timer-second 'display-time-world-timer)) > - (with-current-buffer (get-buffer-create display-time-world-buffer-name) > - (display-time-world-display (time--display-world-list)) > - (display-buffer display-time-world-buffer-name > - (cons nil '((window-height . fit-window-to-buffer)))) > - (display-time-world-mode))) > + (switch-to-buffer-other-window (get-buffer-create display-time-world-buffer-name)) > + (display-time-world-display (time--display-world-list)) > + (display-buffer display-time-world-buffer-name > + (cons nil '((window-height . fit-window-to-buffer)))) switch-to-buffer and friends have historically not played nicely with display-buffer-alist and switch-to-buffer-preserve-window-point when used in Elisp libraries, with adverse user-visible effects. Unless there's a specific reason to use switch-to-buffer, pop-to-buffer and friends should generally be preferred instead. Note that using pop-to-buffer or similar here could remove the need for calling both get-buffer-create and display-buffer, while remaining fully customisable by the user. > + (display-time-world-mode)) > > (defun display-time-world-timer () > (if (get-buffer display-time-world-buffer-name) > -- > 2.26.2 > > > From 47401a1e10213d89cbdb6eaa2b66b44c2be37dfe Mon Sep 17 00:00:00 2001 > From: Stefan Kangas > Date: Sun, 26 Apr 2020 10:47:17 +0200 > Subject: [PATCH 4/4] Improve the display-time-world UI > > * lisp/time.el (display-time-world-buffer-name): Change default > buffer name. > (display-time-world): Add usage instructions to modeline. Are there many other modes that do this? Is it really that helpful to have a standard special-mode binding displayed prominently in the mode line by default, with no easy way to remove it? FWIW, on entry some modes display useful key bindings in the echo area, which I find far less intrusive than a more permanent addition to the mode line. [...] > @@ -570,7 +570,11 @@ display-time-world > (display-time-world-display (time--display-world-list)) > (display-buffer display-time-world-buffer-name > (cons nil '((window-height . fit-window-to-buffer)))) > - (display-time-world-mode)) > + (display-time-world-mode) > + (setq mode-line-format (format "%15s %13s" "World Clock" "q to quit"))) Doesn't this override the entire mode line, erasing all other default constructs? What's wrong with the status quo, which displays the mode name, "World clock", like every other mode does? > +;; This would be a more intuitive name. > +(defalias 'world-clock 'display-time-world) > > (defun display-time-world-timer () > (if (get-buffer display-time-world-buffer-name) > -- > 2.26.2 Otherwise LGTM. -- Basil