From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: "Erik Martin-Dorel" Newsgroups: gmane.emacs.devel Subject: =?utf-8?q?Re=3A?= [nongnu] elpa/opam-switch-mode 8b8670ca8b =?utf-8?q?1/3=3A?==?utf-8?q?_feat(opam-switch-mode-lighter)=3A?= Show the switch name in the mode-bar Date: Tue, 11 Jul 2023 23:09:54 +0200 Message-ID: References: Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="27347"; mail-complaints-to="usenet@ciao.gmane.io" Cc: emacs-devel@gnu.org To: "Stefan Monnier" Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Wed Jul 12 04:25:49 2023 Return-path: Envelope-to: ged-emacs-devel@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 1qJPY5-0006su-4M for ged-emacs-devel@m.gmane-mx.org; Wed, 12 Jul 2023 04:25:49 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qJPXT-0006q4-FK; Tue, 11 Jul 2023 22:25:11 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qJKcU-0005pc-EQ for emacs-devel@gnu.org; Tue, 11 Jul 2023 17:10:02 -0400 Original-Received: from smtp1.irit.fr ([141.115.24.2]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qJKcR-0007hm-Pz for emacs-devel@gnu.org; Tue, 11 Jul 2023 17:10:02 -0400 Original-Received: by wwwap4r8.irit.fr (Postfix, from userid 986) id 410D4C09386; Tue, 11 Jul 2023 23:09:54 +0200 (CEST) In-Reply-To: X-Forward: 90.55.199.236 Received-SPF: pass client-ip=141.115.24.2; envelope-from=Erik.Martin-Dorel@irit.fr; helo=smtp1.irit.fr X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-Mailman-Approved-At: Tue, 11 Jul 2023 22:25:07 -0400 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:307771 Archived-At: Hi Stefan! thanks for your helpful feedback. On Tuesday, July 11, 2023 22:21 CEST, Stefan Monnier wrote: > Salut Erik=C2=B8 >=20 > > + (progn (opam-switch-set-switch ,switch) > > + (redraw-display)) > > :active t > > :help ,(concat "Select opam switch \"" switch "\"")]) > > (opam-switch--get-switches)) > > ;; now reset as last element > > '( > > - ["reset" (opam-switch-set-switch "") > > + ["reset" > > + (progn (opam-switch-set-switch "") > > + (redraw-display)) >=20 > Instead of `redraw-display` you want `force-mode-line-update`. OK thanks. But we specifically need to update the mode-line for *all* f= rames. While `force-mode-line-update` only deals with the current buffer, and `(force-mode-line-update t)` just seems to deal with all buffers fr= om the current frame. Do you confirm? > And you probably want to put it into `opam-switch-set-switch` rather > than duplicating it after every call: Sure: I had done this slight duplication outside of `opam-switch-set-sw= itch` because the need for `redraw-display` only showed up when using the men= u =E2=80=94 direct calls to `M-x opam-switch-set-switch =E2=80=A6` were f= ine. But indeed, your suggestion to get rid of the `progn` looks better! (I'll be able to commit it after you reply to this e-mail :) > Also, in some circumstances the mode lines can be updated fairly ofte= n > (e.g. after every key press, and/or every process output received by > a filter), so it might be worthwhile to memoize/precompute the > lighter rather than recompute it every time the modeline is updated. Thanks for the suggestion, but IINM your patch only precomputes the lig= hter once, while it would need to be recomputed each time we change the opam switc= h, otherwise the ligther becomes out-of-sync. So I'm afraid that this optimization leads to over-complicated code=E2=80= =A6 and given no subexpression of the current (opam-switch-mode-lighter) involves any external system call, I'd say it is not that inefficient/c= ostly=E2=80=A6 so maybe we can just keep it as is; just like the merlin.el lighter is implemented BTW: https://github.com/ocaml/merlin/blob/4f6c7cfee2344dd75e9568f25c0c157652= 1ec049/emacs/merlin.el#L2046 Are you OK with that point? Kind regards, Erik > So maybe the patch below? >=20 >=20 > Stefan >=20 >=20 > diff --git a/opam-switch-mode.el b/opam-switch-mode.el > index 0ec42aa123..b13cbb75d7 100644 > --- a/opam-switch-mode.el > +++ b/opam-switch-mode.el > @@ -93,6 +93,8 @@ background process before the opam switch changes." > =20 > ;;; Code: > =20 > +(defvar opam-switch--mode-lighter nil) > + > (defun opam-switch--run-command-without-stderr (sub-cmd > &optional switch sex= p > &rest args) > @@ -321,6 +323,8 @@ not any other shells outside Emacs." > (unless opam-switch--saved-env > (opam-switch--save-current-env opam-env)) > (opam-switch--set-env opam-env prefix))) > + (setq opam-switch--mode-lighter nil) > + (force-mode-line-update t) > (run-hooks 'opam-switch-change-opam-switch-hook)) > =20 > (define-obsolete-function-alias 'opam-switch--set-switch > @@ -343,17 +347,13 @@ not any other shells outside Emacs." > ;; then the list with all the real opam switches > (mapcar > (lambda (switch) > - `[,switch > - (progn (opam-switch-set-switch ,switch) > - (redraw-display)) > + `[,switch (opam-switch-set-switch ,switch) > :active t > :help ,(concat "Select opam switch \"" switch "\"")]) > (opam-switch--get-switches)) > ;; now reset as last element > '( > - ["reset" > - (progn (opam-switch-set-switch "") > - (redraw-display)) > + ["reset" (opam-switch-set-switch "") > :active opam-switch--saved-env > :help "Reset to state when Emacs was started"]))) > =20 > @@ -376,10 +376,12 @@ is automatically created by `define-minor-mode'= ." > =20 > (defun opam-switch-mode-lighter () > "Return the lighter for opam-switch-mode which indicates the curre= nt switch." > - (let* ((current-switch (opam-switch--get-current-switch)) > - ;; handle the case of local switches for better UX > - (shortened (replace-regexp-in-string ".*/" "=E2=80=A6/" cur= rent-switch t t))) > - (format " OPSW-%s" shortened))) > + (or opam-switch--mode-lighter > + (let* ((current-switch (opam-switch--get-current-switch)) > + ;; handle the case of local switches for better UX > + (shortened (replace-regexp-in-string ".*/" "=E2=80=A6/" > + current-switch t t= ))) > + (setq opam-switch--mode-lighter (format " OPSW-%s" shortened= ))))) > =20 > ;;;###autoload > (define-minor-mode opam-switch-mode --=20 =C3=89rik Martin-Dorel, PhD Ma=C3=AEtre de Conf=C3=A9rences, Lab. IRIT, Univ. Toulouse 3 erik.martin-dorel@irit.fr erik.martin-dorel@master-developpement-logiciel.fr https://www.irit.fr/~Erik.Martin-Dorel/