* bug#27193: 25.2; tmm should use completing-read-default @ 2017-06-02 4:50 Ryan 2017-06-02 15:26 ` Drew Adams 2017-06-03 0:02 ` Glenn Morris 0 siblings, 2 replies; 13+ messages in thread From: Ryan @ 2017-06-02 4:50 UTC (permalink / raw) To: 27193 Below is a patch (formatted using "git format-patch") to avoid using "completing-read" in tmm, instead using "completing-read-default" directly. The rationale is explained in the commit message. Briefly: tmm already pretty much relies on the assumption that completing-read is actually calling completing-read-default. From f184717f9f025a7edc8a015404fef9770233164a Mon Sep 17 00:00:00 2001 From: "Ryan C. Thompson" <rct@thompsonclan.org> Date: Fri, 2 Jun 2017 00:34:41 -0400 Subject: [PATCH] Use completing-read-default in tmm-prompt tmm uses completing-read, but it customizes its behavior so much that any alternative completing-read-function will almost certainly break it. For example, both ido-ubiquitous and ivy have special code to deactivate themselves for tmm. Since tmm is effectively imeplementing its own completion system, it should not depend on the value of completing-read-function. * lisp/tmm.el (tmm-prompt): Use completing-read-default instead of completing-read --- lisp/tmm.el | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lisp/tmm.el b/lisp/tmm.el index c6830903e3..8755971d7c 100644 --- a/lisp/tmm.el +++ b/lisp/tmm.el @@ -247,7 +247,7 @@ Its value should be an event that has a binding in MENU." ;; candidates in the order they are shown on ;; the menu bar. So pass completing-read the ;; reversed copy of the list. - (completing-read + (completing-read-default (concat gl-str " (up/down to change, PgUp to menu): ") (tmm--completion-table (reverse tmm-km-list)) nil t nil -- 2.13.0 In GNU Emacs 25.2.1 (x86_64-apple-darwin13.4.0, NS appkit-1265.21 Version 10.9.5 (Build 13F1911)) of 2017-04-21 built on builder10-9.porkrind.org Windowing system distributor 'Apple', version 10.3.1404 Configured using: 'configure --with-ns '--enable-locallisppath=/Library/Application Support/Emacs/${version}/site-lisp:/Library/Application Support/Emacs/site-lisp' --with-modules' Configured features: NOTIFY ACL GNUTLS LIBXML2 ZLIB TOOLKIT_SCROLL_BARS NS MODULES Important settings: value of $LANG: en_US.UTF-8 locale-coding-system: utf-8-unix Major mode: Magit Log Minor modes in effect: crm-custom-mode: t recentf-mode: t ws-butler-global-mode: t ws-butler-mode: t winner-mode: t sml-modeline-mode: t savehist-mode: t save-place-mode: t minibuffer-electric-default-mode: t minibuffer-depth-indicate-mode: t midnight-mode: t ido-yes-or-no-mode: t highlight-stages-global-mode: t highlight-stages-mode: t global-undo-tree-mode: t undo-tree-mode: t global-pointback-mode: t pointback-mode: t global-hl-line-mode: t global-anzu-mode: t anzu-mode: t desktop-save-mode: t delete-selection-mode: t beacon-mode: t auto-dim-other-buffers-mode: t ido-complete-space-or-hyphen-mode: t ido-ubiquitous-mode: t ido-everywhere: t osx-pseudo-daemon-mode: t diff-auto-refine-mode: t magit-auto-revert-mode: t global-git-commit-mode: t async-bytecomp-package-mode: t autopair-global-mode: t show-paren-mode: t global-auto-complete-mode: t override-global-mode: t shell-dirtrack-mode: t tooltip-mode: t global-eldoc-mode: t electric-indent-mode: t mouse-wheel-mode: t menu-bar-mode: t file-name-shadow-mode: t global-font-lock-mode: t font-lock-mode: t blink-cursor-mode: t auto-composition-mode: t auto-encryption-mode: t auto-compression-mode: t buffer-read-only: t line-number-mode: t transient-mark-mode: t Recent messages: Git finished [2 times] [C-t] show common commands, [?] describe events, [C-h i] show manual Error during redisplay: (jit-lock-function 1) signaled (error "Can’t find the beginning of the file") Error during redisplay: (jit-lock-function 501) signaled (error "Can’t find the beginning of the file") Mark set [2 times] Saved text until "rse tmm-km-list)) nil t nil -- 2.13.0 " Load-path shadows: /Users/ryan/.emacs.d/el-get/ido-completing-read+/ido-completing-read+ hides /Users/ryan/.emacs.d/.cask/25.2/elpa/ido-completing-read+-20170313.1603/ido-completing-read+ /Users/ryan/.emacs.d/.cask/25.2/elpa/org-bullets-20140918.1137/org-bullets hides /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-bullets /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ox hides /Applications/Emacs.app/Contents/Resources/lisp/org/ox /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ox-texinfo hides /Applications/Emacs.app/Contents/Resources/lisp/org/ox-texinfo /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ox-publish hides /Applications/Emacs.app/Contents/Resources/lisp/org/ox-publish /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ox-org hides /Applications/Emacs.app/Contents/Resources/lisp/org/ox-org /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ox-odt hides /Applications/Emacs.app/Contents/Resources/lisp/org/ox-odt /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ox-md hides /Applications/Emacs.app/Contents/Resources/lisp/org/ox-md /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ox-man hides /Applications/Emacs.app/Contents/Resources/lisp/org/ox-man /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ox-latex hides /Applications/Emacs.app/Contents/Resources/lisp/org/ox-latex /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ox-icalendar hides /Applications/Emacs.app/Contents/Resources/lisp/org/ox-icalendar /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ox-html hides /Applications/Emacs.app/Contents/Resources/lisp/org/ox-html /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ox-beamer hides /Applications/Emacs.app/Contents/Resources/lisp/org/ox-beamer /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ox-ascii hides /Applications/Emacs.app/Contents/Resources/lisp/org/ox-ascii /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org hides /Applications/Emacs.app/Contents/Resources/lisp/org/org /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-w3m hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-w3m /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-version hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-version /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-timer hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-timer /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-table hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-table /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-src hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-src /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-rmail hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-rmail /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-protocol hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-protocol /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-plot hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-plot /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-pcomplete hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-pcomplete /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-mouse hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-mouse /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-mobile hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-mobile /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-mhe hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-mhe /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-macs hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-macs /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-macro hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-macro /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-loaddefs hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-loaddefs /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-list hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-list /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-irc hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-irc /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-install hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-install /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-inlinetask hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-inlinetask /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-info hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-info /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-indent hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-indent /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-id hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-id /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-habit hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-habit /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-gnus hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-gnus /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-footnote hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-footnote /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-feed hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-feed /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-faces hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-faces /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-eshell hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-eshell /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-entities hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-entities /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-element hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-element /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-docview hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-docview /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-datetree hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-datetree /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-ctags hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-ctags /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-crypt hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-crypt /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-compat hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-compat /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-colview hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-colview /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-clock hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-clock /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-capture hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-capture /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-bibtex hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-bibtex /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-bbdb hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-bbdb /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-attach hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-attach /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-archive hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-archive /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/org-agenda hides /Applications/Emacs.app/Contents/Resources/lisp/org/org-agenda /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-tangle hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-tangle /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-table hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-table /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-sqlite hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-sqlite /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-sql hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-sql /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-shen hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-shen /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-screen hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-screen /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-scheme hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-scheme /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-scala hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-scala /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-sass hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-sass /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-ruby hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-ruby /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-ref hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-ref /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-R hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-R /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-python hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-python /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-plantuml hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-plantuml /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-picolisp hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-picolisp /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-perl hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-perl /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-org hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-org /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-octave hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-octave /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-ocaml hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-ocaml /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-mscgen hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-mscgen /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-maxima hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-maxima /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-matlab hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-matlab /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-makefile hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-makefile /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-lob hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-lob /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-lisp hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-lisp /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-lilypond hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-lilypond /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-ledger hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-ledger /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-latex hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-latex /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-keys hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-keys /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-js hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-js /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-java hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-java /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-io hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-io /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-haskell hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-haskell /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-gnuplot hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-gnuplot /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-fortran hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-fortran /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-exp hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-exp /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-eval hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-eval /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-emacs-lisp hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-emacs-lisp /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-dot hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-dot /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-ditaa hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-ditaa /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-css hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-css /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-core hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-core /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-comint hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-comint /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-clojure hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-clojure /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-calc hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-calc /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-C hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-C /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-awk hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-awk /Users/ryan/.emacs.d/.cask/25.2/elpa/org-plus-contrib-20170515/ob-asymptote hides /Applications/Emacs.app/Contents/Resources/lisp/org/ob-asymptote /Users/ryan/.emacs.d/.cask/25.2/elpa/seq-2.20/seq hides /Applications/Emacs.app/Contents/Resources/lisp/emacs-lisp/seq /Users/ryan/.emacs.d/.cask/25.2/elpa/let-alist-1.0.5/let-alist hides /Applications/Emacs.app/Contents/Resources/lisp/emacs-lisp/let-alist Features: (shadow sort mail-extr whitespace tmm debug dabbrev colir ivy ivy-overlay ffap crm-custom tabify imenu tramp-cmds tramp-cache webjump woman man eieio-opt speedbar sb-image ezimage dframe misearch multi-isearch recentf tree-widget conf-mode sgml-mode org-eldoc sh-script smie org-rmail org-mhe org-irc org-info org-gnus org-docview doc-view image-mode org-bibtex bibtex org-bbdb org-w3m jka-compr vc-git flymake emacsbug sendmail face-remap ws-butler winner sml-modeline savehist saveplace minibuf-eldef mb-depth midnight ido-yes-or-no icomplete highlight-stages undo-tree diff pointback assoc hl-line anzu desktop frameset delsel beacon auto-dim-other-buffers ido-completing-read+ loadhist bar-cursor debian-changelog-mode git-wip-mode vc vc-dispatcher ido-complete-space-or-hyphen ido-describe-fns ido-ubiquitous tempbuf smooth-scrolling .loaddefs cus-edit cus-start cus-load warnings system-specific-settings snakemake-mode smex ido slime etags xref project arc-mode archive-mode hyperspec python pretty-symbols polymode poly-base polymode-weave polymode-export polymode-debug polymode-methods poly-lock polymode-compat polymode-classes polymode-core eieio-custom wid-edit eieio-base color osx-pseudo-daemon org-bullets occur-context-resize noflet cl-indent markdown-mode thingatpt magit-filenotify magit-obsolete magit-blame magit-stash magit-bisect magit-remote magit-commit magit-sequence magit-notes magit-worktree magit-branch magit-files magit-refs magit-status magit magit-repos magit-apply magit-wip magit-log magit-diff smerge-mode diff-mode magit-core magit-autorevert autorevert filenotify magit-process magit-margin magit-mode magit-git magit-section magit-popup git-commit magit-utils crm log-edit message rfc822 mml mml-sec epg mailabbrev mail-utils gmm-utils mailheader pcvs-util add-log with-editor async-bytecomp async tramp-sh server lexbind-mode highlight-defined header2 git-gutter-fringe fringe-helper git-gutter esup esup-child benchmark ess ess-mode ess-noweb-mode ess-inf ess-tracebug ess-generics ess-utils ess-custom executable ess-compat el-get el-get-autoloading el-get-list-packages el-get-dependencies el-get-build el-get-status pp el-get-methods el-get-fossil el-get-svn el-get-pacman el-get-github-zip el-get-github-tar el-get-http-zip el-get-http-tar el-get-hg el-get-go el-get-git-svn el-get-fink el-get-emacswiki el-get-http el-get-notify el-get-emacsmirror el-get-github el-get-git el-get-elpa el-get-darcs el-get-cvs el-get-bzr el-get-brew el-get-builtin el-get-apt-get el-get-recipes el-get-byte-compile subr-x el-get-custom el-get-core autoload dired creole-mode keydef cperl-mode cl-lib-highlight bs browse-url autopair paren auto-complete edmacro kmacro popup apache-mode adjust-parens exec-path-from-shell use-package diminish bind-key compile org-element avl-tree org org-macro org-footnote org-pcomplete org-list org-faces org-entities noutline outline easy-mmode org-version ob-emacs-lisp ob ob-tangle org-src ob-ref ob-lob ob-table ob-keys ob-exp ob-comint tramp tramp-compat tramp-loaddefs trampver shell pcomplete comint ansi-color ring ob-core ob-eval org-compat org-macs org-loaddefs format-spec find-func cal-menu calendar cal-loaddefs pallet advice gh-common gh-profile url-parse auth-source gnus-util password-cache url-vars marshal eieio-compat ht eieio eieio-core cl slime-autoloads rx info cask cl-seq cl-macs cask-bootstrap package-build mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045 ietf-drums mm-util help-fns mail-prsvr json map lisp-mnt shut-up epl git commander f dash s finder-inf package epg-config seq byte-opt gv bytecomp byte-compile cl-extra help-mode easymenu cconv cl-loaddefs pcase cl-lib time-date mule-util tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type mwheel ns-win ucs-normalize term/common-win tool-bar dnd fontset image regexp-opt fringe tabulated-list newcomment elisp-mode lisp-mode prog-mode register page menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock font-lock syntax facemenu font-core frame cl-generic cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european ethiopic indian cyrillic chinese charscript case-table epa-hook jka-cmpr-hook help simple abbrev minibuffer cl-preloaded nadvice loaddefs button faces cus-face macroexp files text-properties overlay sha1 md5 base64 format env code-pages mule custom widget hashtable-print-readable backquote kqueue cocoa ns multi-tty make-network-process emacs) Memory information: ((conses 16 1730483 82461) (symbols 48 58102 0) (miscs 40 14562 7947) (strings 32 631042 30932) (string-bytes 1 7897000) (vectors 16 90902) (vector-slots 8 2215255 233733) (floats 8 1475 4347) (intervals 56 51527 3760) (buffers 976 169)) ^ permalink raw reply related [flat|nested] 13+ messages in thread
* bug#27193: 25.2; tmm should use completing-read-default 2017-06-02 4:50 bug#27193: 25.2; tmm should use completing-read-default Ryan @ 2017-06-02 15:26 ` Drew Adams 2017-06-02 17:37 ` Ryan Thompson 2017-06-03 0:02 ` Glenn Morris 1 sibling, 1 reply; 13+ messages in thread From: Drew Adams @ 2017-06-02 15:26 UTC (permalink / raw) To: Ryan, 27193 > Below is a patch (formatted using "git format-patch") to avoid using > "completing-read" in tmm, instead using "completing-read-default" > directly. The rationale is explained in the commit message. Briefly: > tmm already pretty much relies on the assumption that completing-read > is actually calling completing-read-default. > > tmm uses completing-read, but it customizes its behavior so much that > any alternative completing-read-function will almost certainly break > it. For example, both ido-ubiquitous and ivy have special code to > deactivate themselves for tmm. Since tmm is effectively imeplementing > its own completion system, it should not depend on the value of > completing-read-function. I don't understand that argument. (But to be clear, I don't really care much about `tmm'.) I don't see that that is an argument for _preventing_ the use of a `completing-read-function' by tmm. I see it only as a statement that it might not be helpful to use some particular values of `completing-read-function' with tmm. If there is a general problem then consider adding a comment in `tmm.el' (or perhaps put it in some doc string) that says what you are really saying: Some values of `completing-read-function' might not be helpful with `tmm', and if you find that is the case, consider binding that variable to nil. But now that I've taken a quick look (just now) at the use by tmm of `completing-read', I don't see that there is a problem to be fixed in `tmm.el'. I don't see that its use of `completing-read' is particularly exotic or problematic. This is it: (completing-read (concat gl-str " (up/down to change, PgUp to menu): ") (tmm--completion-table (reverse tmm-km-list)) nil t nil (cons 'tmm--history (- (* 2 history-len) index-of-default))) I don't see anything at all unusual about that. And the collection function, `tmm--completion-table', is likewise pretty ordinary, I think: (defun tmm--completion-table (items) (lambda (string pred action) (if (eq action 'metadata) '(metadata (display-sort-function . identity)) (complete-with-action action items string pred)))) You say: > tmm already pretty much relies on the assumption that > completing-read is actually calling completing-read-default. I don't see any evidence of that. This kind of argument could (inappropriately, IMO) be applied to any number of completely normal uses of `completing-read'. I see no reason to impose a dichotomy of either a `completing-read-function' similar to yours or else `completing-read-default'. There are likely other benign values of the variable, besides just `completing-read-default'. It sounds like (and no, I haven't looked into it; it just sounds like it) you have some particular `completing-read-function' values in mind, which you have found are incompatible with tmm's use of `completing-read'. If so, that's not an argument for preventing the use of other values of `completing-read-function' with tmm. (Clearly the value `completing-read-default' is fine, for instance.) That's not an argument for tmm to do something to prevent all use of `completing-read-function'. Instead, it's an argument for the code that defines and uses a particular `completing-read-function' to take care of the contexts where it makes sense, and to stop itself from being used in other contexts, where it might cause trouble. Only that code knows the kinds of context where its own `completing-read-function' makes sense and where it does not. Code such as tmm should not try to guess what kinds of trouble different values of `completing-read-function' might cause. I don't think tmm should throw up its hands and say, "Gee, there might be some values of `completing-read-function' that are troublesome, so let's just prevent all use of that variable." That doesn't make sense, to me. If you want additional suggestions, maybe describe just what the problem is that your completion function causes for tmm. It's hard to offer suggestions if you only state that it is incompatible, without going into any detail. (Not that you must ask for input about this. But if you would like some then giving more detail might help.) Please use your own judgment (as I said, I don't really care much about `tmm'), but a priori this sounds like overkill. It sounds a bit like trying to bend Emacs to fit your `completing-read-function'. I can understand such a motivation, believe me; I don't ascribe a bad intention to you. A guess is that you are not sure what to do, to prevent inappropriate application of your value of `completing-read-function' in this or that context. If you think it causes trouble in some contexts, or it is not able to handle some contexts properly, then I'd suggest you consider walling it off from those use cases. It might take some time to discover which contexts it causes trouble for, but that's OK - you could add them as you discover them. Tmm sounds like a start. The right approach, IMO, is to teach your code when to use its `completing-read-function' and when not to use it. Put differently, consider teaching your `completing-read-function' when and where to hold back and just punt to the default behavior. It's obvious that it is possible for someone to create a `completing-read-function' that causes trouble here or there. But such trouble is up to the causer to fix or tame. The approach of preventing code like `tmm.el' from letting other code use `completing-read-function' does not look like it heads in the right direction. But mine is just one opinion. I ask only that you think some more about this. ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#27193: 25.2; tmm should use completing-read-default 2017-06-02 15:26 ` Drew Adams @ 2017-06-02 17:37 ` Ryan Thompson 2017-06-02 20:45 ` Ryan Thompson 2017-06-02 21:25 ` Drew Adams 0 siblings, 2 replies; 13+ messages in thread From: Ryan Thompson @ 2017-06-02 17:37 UTC (permalink / raw) To: Drew Adams, 27193 [-- Attachment #1: Type: text/plain, Size: 9393 bytes --] On Fri, Jun 2, 2017 at 11:26 AM Drew Adams <drew.adams@oracle.com> wrote: > I don't understand that argument. (But to be clear, I don't > really care much about `tmm'.) > Fair enough. My post certainly assumes quite a bit familiarity with tmm. I'm happy to elaborate below. > I don't see that that is an argument for _preventing_ the > use of a `completing-read-function' by tmm. I see it > only as a statement that it might not be helpful to use > some particular values of `completing-read-function' with > tmm. > > If there is a general problem then consider adding a > comment in `tmm.el' (or perhaps put it in some doc > string) that says what you are really saying: Some > values of `completing-read-function' might not be > helpful with `tmm', and if you find that is the case, > consider binding that variable to nil. > > But now that I've taken a quick look (just now) at the > use by tmm of `completing-read', I don't see that there > is a problem to be fixed in `tmm.el'. I don't see that > its use of `completing-read' is particularly exotic or > problematic. This is it: > > (completing-read > (concat gl-str " (up/down to change, PgUp to menu): ") > (tmm--completion-table (reverse tmm-km-list)) nil t nil > (cons 'tmm--history (- (* 2 history-len) index-of-default))) > > I don't see anything at all unusual about that. > > And the collection function, `tmm--completion-table', > is likewise pretty ordinary, I think: > > (defun tmm--completion-table (items) > (lambda (string pred action) > (if (eq action 'metadata) > '(metadata (display-sort-function . identity)) > (complete-with-action action items string pred)))) > > You say: > > > tmm already pretty much relies on the assumption that > > completing-read is actually calling completing-read-default. > > I don't see any evidence of that. > tmm's weirdness is not in the actual call to completing-read. That completing-read call is wrapped by "(minibuffer-with-setup-hook #'tmm-add-prompt ...)", which in turn calls tmm-define-keys, which sets up a bunch of non-standard key bindings, which have the effect of implementing a menu system with single-keypress activation of menu items, rather than completion of a substring with string matching. The result is not even recognizable as completing-read. The use of completing-read is merely an implementation detail in a system that is *not* actually doing completion of any kind. So pluggable completion backends don't make any sense for tmm, and I can't imagine any other value of completing-read-function that would make sense for tmm besides completing-read-default. Looking at the "git blame" output for tmm, that call to completing-read has definitely not been updated since completing-read-function was introduced except for minor bugfixes, so it makes sense that tmm would be expecting one and only one implementation of completing-read. > This kind of argument could (inappropriately, IMO) be > applied to any number of completely normal uses of > `completing-read'. > > I see no reason to impose a dichotomy of either a > `completing-read-function' similar to yours or else > `completing-read-default'. There are likely other > benign values of the variable, besides just > `completing-read-default'. > I'm not trying to set a general precedent here. tmm is the only code that I'm aware of that uses completing-read in this way. It sounds like (and no, I haven't looked into it; > it just sounds like it) you have some particular > `completing-read-function' values in mind, which > you have found are incompatible with tmm's use of > `completing-read'. > The alternative completing-read-function providers that I am aware of are of are ido-ubiquitous (written by me), ivy, and helm. I'll also include icicles, even though uses some other mechanism besides modifying completing-read-function. ido-ubiquitous and ivy both have code to deactivate themselves when completing-read is called by tmm because otherwise their completion systems would break it, while icicles and helm simply break tmm when enabled. Thus, to my knowledge there is currently no other completing-read-function that doesn't break tmm (except for those that make exceptions specifically for tmm). If so, that's not an argument for preventing the use of > other values of `completing-read-function' with tmm. > (Clearly the value `completing-read-default' is fine, > for instance.) That's not an argument for tmm to do > something to prevent all use of `completing-read-function'. > > Instead, it's an argument for the code that defines and > uses a particular `completing-read-function' to take > care of the contexts where it makes sense, and to stop > itself from being used in other contexts, where it might > cause trouble. > > Only that code knows the kinds of context where its own > `completing-read-function' makes sense and where it does > not. Code such as tmm should not try to guess what kinds > of trouble different values of `completing-read-function' > might cause. > > I don't think tmm should throw up its hands and say, "Gee, > there might be some values of `completing-read-function' > that are troublesome, so let's just prevent all use of > that variable." That doesn't make sense, to me. > Based on my explanation above, that is precisely what I think tmm should do: avoid using completing-read-function entirely. To look at it another way, tmm was originally written to use completing-read as an implementation detail, and later the function that used to be called completing-read was renamed to completing-read-default, but tmm was never updated to use the new name. This patch rectifies that. > If you want additional suggestions, maybe describe just > what the problem is that your completion function causes > for tmm. It's hard to offer suggestions if you only > state that it is incompatible, without going into any > detail. (Not that you must ask for input about this. > But if you would like some then giving more detail might > help.) > > Please use your own judgment (as I said, I don't really > care much about `tmm'), but a priori this sounds like > overkill. > > It sounds a bit like trying to bend Emacs to fit your > `completing-read-function'. I can understand such a > motivation, believe me; I don't ascribe a bad intention > to you. A guess is that you are not sure what to do, > to prevent inappropriate application of your value of > `completing-read-function' in this or that context. > > If you think it causes trouble in some contexts, or it > is not able to handle some contexts properly, then > I'd suggest you consider walling it off from those use > cases. It might take some time to discover which > contexts it causes trouble for, but that's OK - you > could add them as you discover them. Tmm sounds like > a start. > The right approach, IMO, is to teach your code when to > use its `completing-read-function' and when not to use > it. Put differently, consider teaching your > `completing-read-function' when and where to hold back > and just punt to the default behavior. > This is exactly how ido-ubiquitous and ivy both currently work: they essentially have a blacklist of callers for which they revert to standard completion. tmm is on the blacklist for both packages. Certainly, for any alternative completion system there will be cases where it needs to fall back to standard completion. In my view, the completion system should be able to determine purely based on the calling context (i.e. its arguments and any relevant dynamically-bound variables) whether or not it needs to punt. Making this decision based on the name of the caller instead of the context to make this decision is admitting that not enough context was provided. I view it as a workaround, not a desirable design pattern, and someday in the future I hope to be able to remove the blacklist from ido-ubiquitous. In the case of tmm, the best heuristic I can think of would be to inspect the key bindings of all the letters and numbers. If any of them are locally rebound in the minibuffer to something other than self-insert-command, then punt. However, this doesn't work in practice because the bindings happen in minibuffer-setup-hook, so putting a check at the front of this hook is too early, and putting it at the end is too late because (in the case of ido-ubiquitous) an error is triggered before reaching the end of the hook. This was partly my motivation for suggesting the change in tmm rather than working around it in ido-ubiquitous: because the blacklist approach is the only way for ido-ubiquitous to fix it. It's obvious that it is possible for someone to create > a `completing-read-function' that causes trouble here > or there. But such trouble is up to the causer to fix > or tame. > For the reasons described above, I would be very surprised if there was *any* alternative completion system that didn't break tmm. The approach of preventing code like `tmm.el' from > letting other code use `completing-read-function' does > not look like it heads in the right direction. But > mine is just one opinion. I ask only that you think > some more about this. > As mentioned above, tmm is the only code I'm aware of that does anything like this, and I don't see this as a general approach to be applied anywhere else. Hopefully the above clarifies my rationale in proposing this change. [-- Attachment #2: Type: text/html, Size: 11671 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#27193: 25.2; tmm should use completing-read-default 2017-06-02 17:37 ` Ryan Thompson @ 2017-06-02 20:45 ` Ryan Thompson 2017-06-02 21:25 ` Drew Adams 1 sibling, 0 replies; 13+ messages in thread From: Ryan Thompson @ 2017-06-02 20:45 UTC (permalink / raw) To: Drew Adams, 27193 [-- Attachment #1: Type: text/plain, Size: 10056 bytes --] I should revise what I said slightly. Alternate completion implementations might never be able to do away completely with blacklists, because things like read-file-name also use completing-read. Still, I stand by my statement that nothing else I'm aware of uses completing-read in the same way as tmm. On Fri, Jun 2, 2017 at 1:37 PM Ryan Thompson <rct@thompsonclan.org> wrote: > On Fri, Jun 2, 2017 at 11:26 AM Drew Adams <drew.adams@oracle.com> wrote: > >> I don't understand that argument. (But to be clear, I don't >> really care much about `tmm'.) >> > > Fair enough. My post certainly assumes quite a bit familiarity with tmm. > I'm happy to elaborate below. > > >> I don't see that that is an argument for _preventing_ the >> use of a `completing-read-function' by tmm. I see it >> only as a statement that it might not be helpful to use >> some particular values of `completing-read-function' with >> tmm. >> >> If there is a general problem then consider adding a >> comment in `tmm.el' (or perhaps put it in some doc >> string) that says what you are really saying: Some >> values of `completing-read-function' might not be >> helpful with `tmm', and if you find that is the case, >> consider binding that variable to nil. >> >> But now that I've taken a quick look (just now) at the >> > use by tmm of `completing-read', I don't see that there >> is a problem to be fixed in `tmm.el'. I don't see that >> its use of `completing-read' is particularly exotic or >> problematic. This is it: >> >> (completing-read >> (concat gl-str " (up/down to change, PgUp to menu): ") >> (tmm--completion-table (reverse tmm-km-list)) nil t nil >> (cons 'tmm--history (- (* 2 history-len) index-of-default))) >> >> I don't see anything at all unusual about that. >> >> And the collection function, `tmm--completion-table', >> is likewise pretty ordinary, I think: >> >> (defun tmm--completion-table (items) >> (lambda (string pred action) >> (if (eq action 'metadata) >> '(metadata (display-sort-function . identity)) >> (complete-with-action action items string pred)))) >> >> You say: >> >> > tmm already pretty much relies on the assumption that >> > completing-read is actually calling completing-read-default. >> >> I don't see any evidence of that. >> > > tmm's weirdness is not in the actual call to completing-read. That > completing-read call is wrapped by "(minibuffer-with-setup-hook > #'tmm-add-prompt ...)", which in turn calls tmm-define-keys, which sets up > a bunch of non-standard key bindings, which have the effect of implementing > a menu system with single-keypress activation of menu items, rather than > completion of a substring with string matching. The result is not even > recognizable as completing-read. The use of completing-read is merely an > implementation detail in a system that is *not* actually doing completion > of any kind. So pluggable completion backends don't make any sense for tmm, > and I can't imagine any other value of completing-read-function that would > make sense for tmm besides completing-read-default. Looking at the "git > blame" output for tmm, that call to completing-read has definitely not been > updated since completing-read-function was introduced except for minor > bugfixes, so it makes sense that tmm would be expecting one and only one > implementation of completing-read. > > >> This kind of argument could (inappropriately, IMO) be >> applied to any number of completely normal uses of >> `completing-read'. >> >> I see no reason to impose a dichotomy of either a >> `completing-read-function' similar to yours or else >> `completing-read-default'. There are likely other >> benign values of the variable, besides just >> `completing-read-default'. >> > > I'm not trying to set a general precedent here. tmm is the only code that > I'm aware of that uses completing-read in this way. > > It sounds like (and no, I haven't looked into it; >> it just sounds like it) you have some particular >> `completing-read-function' values in mind, which >> you have found are incompatible with tmm's use of >> `completing-read'. >> > > The alternative completing-read-function providers that I am aware of are > of are ido-ubiquitous (written by me), ivy, and helm. I'll also include > icicles, even though uses some other mechanism besides modifying > completing-read-function. ido-ubiquitous and ivy both have code to > deactivate themselves when completing-read is called by tmm because > otherwise their completion systems would break it, while icicles and helm > simply break tmm when enabled. Thus, to my knowledge there is currently no > other completing-read-function that doesn't break tmm (except for those > that make exceptions specifically for tmm). > > If so, that's not an argument for preventing the use of >> other values of `completing-read-function' with tmm. >> (Clearly the value `completing-read-default' is fine, >> for instance.) That's not an argument for tmm to do >> something to prevent all use of `completing-read-function'. >> >> Instead, it's an argument for the code that defines and >> uses a particular `completing-read-function' to take >> care of the contexts where it makes sense, and to stop >> itself from being used in other contexts, where it might >> cause trouble. >> >> Only that code knows the kinds of context where its own >> `completing-read-function' makes sense and where it does >> not. Code such as tmm should not try to guess what kinds >> of trouble different values of `completing-read-function' >> might cause. >> >> I don't think tmm should throw up its hands and say, "Gee, >> there might be some values of `completing-read-function' >> that are troublesome, so let's just prevent all use of >> that variable." That doesn't make sense, to me. >> > > Based on my explanation above, that is precisely what I think tmm should > do: avoid using completing-read-function entirely. To look at it another > way, tmm was originally written to use completing-read as an implementation > detail, and later the function that used to be called completing-read was > renamed to completing-read-default, but tmm was never updated to use the > new name. This patch rectifies that. > > >> If you want additional suggestions, maybe describe just >> what the problem is that your completion function causes >> for tmm. It's hard to offer suggestions if you only >> state that it is incompatible, without going into any >> detail. (Not that you must ask for input about this. >> But if you would like some then giving more detail might >> help.) >> >> Please use your own judgment (as I said, I don't really >> care much about `tmm'), but a priori this sounds like >> overkill. >> >> It sounds a bit like trying to bend Emacs to fit your >> `completing-read-function'. I can understand such a >> motivation, believe me; I don't ascribe a bad intention >> to you. A guess is that you are not sure what to do, >> to prevent inappropriate application of your value of >> `completing-read-function' in this or that context. >> >> If you think it causes trouble in some contexts, or it >> is not able to handle some contexts properly, then >> I'd suggest you consider walling it off from those use >> cases. It might take some time to discover which >> contexts it causes trouble for, but that's OK - you >> could add them as you discover them. Tmm sounds like >> a start. > > >> The right approach, IMO, is to teach your code when to >> use its `completing-read-function' and when not to use >> it. Put differently, consider teaching your >> `completing-read-function' when and where to hold back >> and just punt to the default behavior. >> > > This is exactly how ido-ubiquitous and ivy both currently work: they > essentially have a blacklist of callers for which they revert to standard > completion. tmm is on the blacklist for both packages. Certainly, for any > alternative completion system there will be cases where it needs to fall > back to standard completion. In my view, the completion system should be > able to determine purely based on the calling context (i.e. its arguments > and any relevant dynamically-bound variables) whether or not it needs to > punt. Making this decision based on the name of the caller instead of the > context to make this decision is admitting that not enough context was > provided. I view it as a workaround, not a desirable design pattern, and > someday in the future I hope to be able to remove the blacklist from > ido-ubiquitous. > > In the case of tmm, the best heuristic I can think of would be to inspect > the key bindings of all the letters and numbers. If any of them are locally > rebound in the minibuffer to something other than self-insert-command, then > punt. However, this doesn't work in practice because the bindings happen in > minibuffer-setup-hook, so putting a check at the front of this hook is too > early, and putting it at the end is too late because (in the case of > ido-ubiquitous) an error is triggered before reaching the end of the hook. > This was partly my motivation for suggesting the change in tmm rather than > working around it in ido-ubiquitous: because the blacklist approach is the > only way for ido-ubiquitous to fix it. > > It's obvious that it is possible for someone to create >> a `completing-read-function' that causes trouble here >> or there. But such trouble is up to the causer to fix >> or tame. >> > > For the reasons described above, I would be very surprised if there was > *any* alternative completion system that didn't break tmm. > > The approach of preventing code like `tmm.el' from >> letting other code use `completing-read-function' does >> not look like it heads in the right direction. But >> mine is just one opinion. I ask only that you think >> some more about this. >> > > As mentioned above, tmm is the only code I'm aware of that does anything > like this, and I don't see this as a general approach to be applied > anywhere else. > > Hopefully the above clarifies my rationale in proposing this change. > [-- Attachment #2: Type: text/html, Size: 13082 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#27193: 25.2; tmm should use completing-read-default 2017-06-02 17:37 ` Ryan Thompson 2017-06-02 20:45 ` Ryan Thompson @ 2017-06-02 21:25 ` Drew Adams 2017-06-02 21:37 ` Dmitry Gutov 2017-06-02 22:52 ` Ryan Thompson 1 sibling, 2 replies; 13+ messages in thread From: Drew Adams @ 2017-06-02 21:25 UTC (permalink / raw) To: Ryan Thompson, 27193 [-- Attachment #1: Type: text/plain, Size: 15451 bytes --] I don't understand that argument. (But to be clear, I don't really care much about `tmm'.) Fair enough. My post certainly assumes quite a bit familiarity with tmm. I'm happy to elaborate below. I am familiar with tmm. I don't care much about it (or for it). I use HYPERLINK "https://www.emacswiki.org/emacs/LaCarte"LaCarte instead. I don't see that its use of `completing-read' is particularly exotic or problematic. This is it: (completing-read ...) I don't see anything at all unusual about that. And the collection function, `tmm--completion-table', is likewise pretty ordinary, I think... You say: > tmm already pretty much relies on the assumption that > completing-read is actually calling completing-read-default. I don't see any evidence of that. tmm's weirdness is not in the actual call to completing-read. That completing-read call is wrapped by "(minibuffer-with-setup-hook #'tmm-add-prompt ...)", which in turn calls tmm-define-keys, which sets up a bunch of non-standard key bindings, which have the effect of implementing a menu system with single-keypress activation of menu items, rather than completion of a substring with string matching. I could have added that to the list of things that I do not find extraordinary or problematic in general for `completing-read-function': the key bindings, the completion function, all of it. The case that needs to be made for your proposal is really (IMO) that there are NO values of `completing-read-function', apart from `completing-read-default', which would be compatible with the tmm code. I don't think you can make that case. I sense you are extrapolating from your own particular use of `completing-read-function' (and similar uses). The result is not even recognizable as completing-read. I don't think so. Certainly no more unrecognizable than Ido, which doesn't use *Completions*, defines its own keys, etc. There is lots of room for different uses of `completing-read', and some of those uses might not look much like the behavior of `completing-read-default'. Nothing wrong with that, in itself. And whether a user might or might not realize that `completing-read' is being used is not relevant, IMO. And your proposed fix is simply to impose `completing-read-default', which hardly supports your argument that tmm provides some kind of anomalous, non-completing-read `completing-read'. `completing-read-default' is the canonical `completing-read' behavior, if there ever was one. And that's the behavior that tmm provides by default. The use of completing-read is merely an implementation detail How so? Please think about providing a different implementation of tmm, which doesn't use `completing-read'. That will let us know whether `completing-read' is used only as an implementation detail. in a system that is *not* actually doing completion of any kind. Of course it does completion. Erase chars in the minibuffer from the right and then hit TAB - completion. It's not a completion to write home about, but it's completion. So pluggable completion backends don't make any sense for tmm, That too sounds overly broad. I think that you have too readily convinced yourself that this and that don't make any possible sense, and you are prone to nail things down to prevent what you see as the nonsensical. If such were truly nonsensical then they would likely never exist, and there would be no reason to forbid them. I sense that you think of "pluggable completion backends", that is, uses of `completing-read-function', as necessarily something similar to what your code does. [FWIW I don't think of `completing-read-function' as providing only a "pluggable completion backend". Do you think of `isearch-search-fun-function' as a "pluggable search backend"? Backend? frontend? There are many angles to something like completion or search or... There is not just a backend and a frontend. Likewise, what you call an "alternative completion system". Really, we're just talking about a completion function - a value for `completing-read-function'. That has wide scope - there are few constraints on what it must do. I don't see the justice in adding a constraint for tmm that it cannot do anything except `completing-read-default'.] and I can't imagine any other value of completing-read-function that would make sense for tmm besides completing-read-default. All you need to imagine is something similar to `completing-read-default' but slightly different in some respect. That's all you need to imagine, but nothing prevents more imagination. And what you or I can imagine is not really the point. The question is whether tmm.el must limit itself to `completing-read-default'. I don't see why it should, just because we know of some values of `completing-read-function' that don't do the right thing for tmm. Looking at the "git blame" output for tmm, that call to completing-read has definitely not been updated since completing-read-function was introduced except for minor bugfixes, That's irrelevant. so it makes sense that tmm would be expecting one and only one implementation of completing-read. That does not follow at all. This kind of argument could (inappropriately, IMO) be applied to any number of completely normal uses of `completing-read'. I see no reason to impose a dichotomy of either a `completing-read-function' similar to yours or else `completing-read-default'. There are likely other benign values of the variable, besides just `completing-read-default'. I'm not trying to set a general precedent here. tmm is the only code that I'm aware of that uses completing-read in this way. I agree that it is not a common way of using it. It doesn't follow that the only possible value of `completing-read-function' that is compatible with tmm is `completing-read-default'. Not at all. It sounds like (and no, I haven't looked into it; it just sounds like it) you have some particular `completing-read-function' values in mind, which you have found are incompatible with tmm's use of `completing-read'. The alternative completing-read-function providers that I am aware of are of are ido-ubiquitous (written by me), ivy, and helm. I'll also include icicles, even though uses some other mechanism besides modifying completing-read-function. ido-ubiquitous and ivy both have code to deactivate themselves when completing-read is called by tmm because otherwise their completion systems would break it, while icicles and helm simply break tmm when enabled. Thus, to my knowledge there is currently no other completing-read-function that doesn't break tmm (except for those that make exceptions specifically for tmm). Again, it is irrelevant that there are uses of `completing-read-function' that break tmm. And what you or I am aware of, or even what might exist anywhere today, does not define the scope of possibilities. [I drink values of the variable `liquid'. Some values, such as strong sulfuric acid, are quite incompatible with my proper functioning. That doesn't mean that the only possible value or the only compatible value for me is the default value, `water'. I drink blindly - I don't know what's in the glass. The only requirement my mouth imposes is that the variable value be a liquid. It is up to whoever fills my glass to DTRT.] And if those uses of `completing-read-function' are incompatible with tmm, and they thus deactivate themselves for tmm commands, that is exactly the right approach, IMO. It is exactly that which I suggested to you (without knowing that that is what you already do). [Tmm does work with Icicles, BTW. (But Icicles does not use `completing-read-function', among other reasons because it wants to work also with older Emacs versions.)] If so, that's not an argument for preventing the use of other values of `completing-read-function' with tmm. (Clearly the value `completing-read-default' is fine, for instance.) That's not an argument for tmm to do something to prevent all use of `completing-read-function'. Instead, it's an argument for the code that defines and uses a particular `completing-read-function' to take care of the contexts where it makes sense, and to stop itself from being used in other contexts, where it might cause trouble. Only that code knows the kinds of context where its own `completing-read-function' makes sense and where it does not. Code such as tmm should not try to guess what kinds of trouble different values of `completing-read-function' might cause. I don't think tmm should throw up its hands and say, "Gee, there might be some values of `completing-read-function' that are troublesome, so let's just prevent all use of that variable." That doesn't make sense, to me. Based on my explanation above, that is precisely what I think tmm should do: avoid using completing-read-function entirely. I know you think that, but I don't see why. There are surely values of `completing-read-function' that do not bother tmm. We know of one already: `completing-read-default'. Why would you suppose that there can be no others? To look at it another way, tmm was originally written to use completing-read as an implementation detail, and later the function that used to be called completing-read was renamed to completing-read-default, but tmm was never updated to use the new name. This patch rectifies that. That's completely imagination. `completing-read-default' is not the new name of what was once `completing-read'. `completing-read' has never used code like `completing-read-default'. (It has always been written in C.) What I would say that perhaps you will think goes a bit in your direction is this: If you can rewrite `tmm.el' so that it has the same behavior without using `completing-read', and the code is at least as simple and easy to maintain, then I'd say go ahead and do that. That would support your idea that `completing-read' is only an implementation detail etc., and the question of `completing-read-function' would become moot. If you want additional suggestions, maybe describe just what the problem is that your completion function causes for tmm. It's hard to offer suggestions if you only state that it is incompatible, without going into any detail. (Not that you must ask for input about this. But if you would like some then giving more detail might help.) Please use your own judgment (as I said, I don't really care much about `tmm'), but a priori this sounds like overkill. It sounds a bit like trying to bend Emacs to fit your `completing-read-function'. I can understand such a motivation, believe me; I don't ascribe a bad intention to you. A guess is that you are not sure what to do, to prevent inappropriate application of your value of `completing-read-function' in this or that context. If you think it causes trouble in some contexts, or it is not able to handle some contexts properly, then I'd suggest you consider walling it off from those use cases. It might take some time to discover which contexts it causes trouble for, but that's OK - you could add them as you discover them. Tmm sounds like a start. The right approach, IMO, is to teach your code when to use its `completing-read-function' and when not to use it. Put differently, consider teaching your `completing-read-function' when and where to hold back and just punt to the default behavior. This is exactly how ido-ubiquitous and ivy both currently work: they essentially have a blacklist of callers for which they revert to standard completion. tmm is on the blacklist for both packages. That's great. Problem solved. That's the right approach, IMO. Certainly, for any alternative completion system Any? Again with the superlatives. There is more to the world... there will be cases where it needs to fall back to standard completion. In my view, the completion system should be able to determine purely based on the calling context (i.e. its arguments and any relevant dynamically-bound variables) whether or not it needs to punt. Making this decision based on the name of the caller instead of the context to make this decision is admitting that not enough context was provided. I view it as a workaround, not a desirable design pattern, and someday in the future I hope to be able to remove the blacklist from ido-ubiquitous. In the case of tmm, the best heuristic I can think of would be to inspect the key bindings of all the letters and numbers. If any of them are locally rebound in the minibuffer to something other than self-insert-command, then punt. That would be silly, IMHO. There can be plenty of uses of `completing-read' where letter or number keys are bound to commands other than `self-insert-command'. [FWIW, though not directly relevant, when Icicle mode is on there are no keys bound to `self-insert-command' in any of the minibuffer completion keymaps. (But there are keys bound to `icicle-self-insert'.)] However, this doesn't work in practice because the bindings happen in minibuffer-setup-hook, so putting a check at the front of this hook is too early, and putting it at the end is too late because (in the case of ido-ubiquitous) an error is triggered before reaching the end of the hook. This was partly my motivation for suggesting the change in tmm rather than working around it in ido-ubiquitous: because the blacklist approach is the only way for ido-ubiquitous to fix it. It sounds like it is already doing things the right way. (And yes, it might mean a slight maintenance chore for you if libraries such as tmm change significantly.) It's obvious that it is possible for someone to create a `completing-read-function' that causes trouble here or there. But such trouble is up to the causer to fix or tame. For the reasons described above, I would be very surprised if there was *any* alternative completion system that didn't break tmm. Just pick a value of `completing-read-function' that is only slightly different from `completing-read-default', to convince yourself otherwise. I really think you have your particular use case too much in mind, here. `completing-read-function' just means something that uses the same arguments as `completing-read-default' and does something somewhat similar. Its marching orders are pretty general. The approach of preventing code like `tmm.el' from letting other code use `completing-read-function' does not look like it heads in the right direction. But mine is just one opinion. I ask only that you think some more about this. As mentioned above, tmm is the only code I'm aware of that does anything like this, and I don't see this as a general approach to be applied anywhere else. OK, that's reassuring. Hopefully the above clarifies my rationale in proposing this change. Thanks for clarifying. I hope my comments above clarify my point of view also, and I hope they help. [-- Attachment #2: Type: text/html, Size: 31728 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#27193: 25.2; tmm should use completing-read-default 2017-06-02 21:25 ` Drew Adams @ 2017-06-02 21:37 ` Dmitry Gutov 2017-06-02 22:19 ` Drew Adams 2017-06-02 22:52 ` Ryan Thompson 1 sibling, 1 reply; 13+ messages in thread From: Dmitry Gutov @ 2017-06-02 21:37 UTC (permalink / raw) To: Drew Adams, Ryan Thompson, 27193 On 6/3/17 12:25 AM, Drew Adams wrote: > The case that needs to be made for your proposal is really (IMO) that > there are /NO/ values of `completing-read-function', apart from > `completing-read-default', which would be compatible with the tmm code. That would be too strong. > I don't think you can make that case. I sense you are extrapolating from > your own particular use of `completing-read-function' (and similar uses). This is ridiculous. If there are functions that adhere to the contract of completing-read-function, and tmm can't work with them, tmm should use completing-read-default, and that's that. I'll install the submitted patch in a few days if someone doesn't beat me to it. > The result is not even recognizable as completing-read. > > I don't think so. Certainly no more unrecognizable than Ido... You are mixing up concepts right and left. E.g. APIs with their callers. ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#27193: 25.2; tmm should use completing-read-default 2017-06-02 21:37 ` Dmitry Gutov @ 2017-06-02 22:19 ` Drew Adams 2017-06-02 22:24 ` Dmitry Gutov 2017-06-02 23:08 ` Ryan Thompson 0 siblings, 2 replies; 13+ messages in thread From: Drew Adams @ 2017-06-02 22:19 UTC (permalink / raw) To: Dmitry Gutov, Ryan Thompson, 27193 > > The case that needs to be made for your proposal is really (IMO) that > > there are /NO/ values of `completing-read-function', apart from > > `completing-read-default', which would be compatible with the tmm code. > > That would be too strong. No. That's exactly what you're forcing, if you force `completing-read-default' as the ONLY possible value. Precisely that: you allow no other values. The claim behind that must be that ALL other values would be problematic and so must be disallowed. > > I don't think you can make that case. I sense you > > are extrapolating from your own particular use of > > `completing-read-function' (and similar uses). > > This is ridiculous. If there are functions that adhere to the contract > of completing-read-function, and tmm can't work with them, tmm should > use completing-read-default, and that's that. "Adhere to the contract of `completing-read-function'"? Are you kidding? What contract is that? (defvar completing-read-function 'completing-read-default "The function called by `completing-read' to do its work. It should accept the same arguments as `completing-read'.") Just accept the arguments - that's the only contract. "Do the work" of `completing-read' is completely vague. That means that anytime you can come up with a function that accepts those args and causes trouble, `completing-read-default' should be imposed as the only possible value. THAT is ridiculous. > I'll install the submitted patch in a few days if someone > doesn't beat me to it. Of course you will. > > The result is not even recognizable as completing-read. > > > > I don't think so. Certainly no more unrecognizable than Ido... > > You are mixing up concepts right and left. E.g. APIs with their callers. Nonsense. Ido's completion behavior is as far from that of vanilla `completing-read' as is Tmm's completion behavior ("the result"). ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#27193: 25.2; tmm should use completing-read-default 2017-06-02 22:19 ` Drew Adams @ 2017-06-02 22:24 ` Dmitry Gutov 2017-06-02 23:08 ` Ryan Thompson 1 sibling, 0 replies; 13+ messages in thread From: Dmitry Gutov @ 2017-06-02 22:24 UTC (permalink / raw) To: Drew Adams, Ryan Thompson, 27193 On 6/3/17 1:19 AM, Drew Adams wrote: > "Adhere to the contract of `completing-read-function'"? > Are you kidding? What contract is that? So the docstring is not written out fully and seriously enough. Even so, it's not hard to understand. > (defvar completing-read-function 'completing-read-default > "The function called by `completing-read' to do its work. > It should accept the same arguments as `completing-read'.") > > Just accept the arguments - that's the only contract. > "Do the work" of `completing-read' is completely vague. Yup. If the value of this variable accepts the arguments and performs completion, > That means that anytime you can come up with a function that > accepts those args and causes trouble, `completing-read-default' > should be imposed as the only possible value. Pretty much. Except we don't consider the very extremes, like completer functions trying to break examples like this intentionally. So not "a function" but "any reasonable function". It's Emacs, after all. >> I'll install the submitted patch in a few days if someone >> doesn't beat me to it. > > Of course you will. Thanks for the encouragement. >>> The result is not even recognizable as completing-read. >>> >>> I don't think so. Certainly no more unrecognizable than Ido... >> >> You are mixing up concepts right and left. E.g. APIs with their callers. > > Nonsense. Ido's completion behavior is as far from that of > vanilla `completing-read' as is Tmm's completion behavior > ("the result"). Ido is one of the API's possible implementations. TMM is its caller. Comparing them doesn't make any sense. ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#27193: 25.2; tmm should use completing-read-default 2017-06-02 22:19 ` Drew Adams 2017-06-02 22:24 ` Dmitry Gutov @ 2017-06-02 23:08 ` Ryan Thompson 2017-06-02 23:10 ` Ryan Thompson 1 sibling, 1 reply; 13+ messages in thread From: Ryan Thompson @ 2017-06-02 23:08 UTC (permalink / raw) To: Drew Adams, Dmitry Gutov, 27193 [-- Attachment #1: Type: text/plain, Size: 867 bytes --] On Fri, Jun 2, 2017 at 6:19 PM Drew Adams <drew.adams@oracle.com> wrote: > Ido's completion behavior is as far from that of > vanilla `completing-read' as is Tmm's completion behavior > ("the result"). > This is kind of my point. If someone calls ido-completing-read, you wouldn't expect it to do something different based on the value of completing-read-function, even if it ido used completing-read internally (which it might have actually done in the past, but currently does not), because by calling ido-completing-read the code has already specified it wants ido completion. Similarly, tmm is implementing a very different behavior from completing-read that is only recognizable as regular completion if you specifically go looking for leaks in the abstraction, and for the same reason I don't think tmm should be paying attention to completing-read-function. [-- Attachment #2: Type: text/html, Size: 1175 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#27193: 25.2; tmm should use completing-read-default 2017-06-02 23:08 ` Ryan Thompson @ 2017-06-02 23:10 ` Ryan Thompson 0 siblings, 0 replies; 13+ messages in thread From: Ryan Thompson @ 2017-06-02 23:10 UTC (permalink / raw) To: Drew Adams, Dmitry Gutov, 27193 [-- Attachment #1: Type: text/plain, Size: 1143 bytes --] Or to put the same thing in a more user-centric context, I think users would be surprised if changing the value of completing-read-function changed the behavior of tmm. On Fri, Jun 2, 2017 at 7:08 PM Ryan Thompson <rct@thompsonclan.org> wrote: > On Fri, Jun 2, 2017 at 6:19 PM Drew Adams <drew.adams@oracle.com> wrote: > >> Ido's completion behavior is as far from that of >> vanilla `completing-read' as is Tmm's completion behavior >> ("the result"). >> > > This is kind of my point. If someone calls ido-completing-read, you > wouldn't expect it to do something different based on the value of > completing-read-function, even if it ido used completing-read internally > (which it might have actually done in the past, but currently does not), > because by calling ido-completing-read the code has already specified it > wants ido completion. Similarly, tmm is implementing a very different > behavior from completing-read that is only recognizable as regular > completion if you specifically go looking for leaks in the abstraction, and > for the same reason I don't think tmm should be paying attention to > completing-read-function. > [-- Attachment #2: Type: text/html, Size: 1721 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#27193: 25.2; tmm should use completing-read-default 2017-06-02 21:25 ` Drew Adams 2017-06-02 21:37 ` Dmitry Gutov @ 2017-06-02 22:52 ` Ryan Thompson 2017-06-03 0:15 ` Drew Adams 1 sibling, 1 reply; 13+ messages in thread From: Ryan Thompson @ 2017-06-02 22:52 UTC (permalink / raw) To: Drew Adams, 27193 [-- Attachment #1: Type: text/plain, Size: 14705 bytes --] (Note: I think several more replies came in while I was writing this; I haven't read those yet.) Ok, so if I'm understanding correctly, the thrust of your argument is that completing-read is a very general function that supports a very wide range of behaviors/usages, to such a degree that tmm's usage does not seem like an anomalous or unusual way to use it. I might not agree on what constitutes a typical usage of completing-read, but I can definitely agree that it is a very general function, and in practice no alternative value of completing-read-function that works substantially differently from completing-read-default is likely to be capable of replicating all of that functionality. So given your recommendation that alternative completion functions should "punt" cases they can't handle back to completing-read-default, along with the high likelihood that every alternate completion system is going to have something that it can't handle and needs to punt on, would you be interested in a patch that implements that punting mechanism within completing-read itself, so that ido-ubiqutous, ivy, helm, and others don't all have to invent their own punting code? I'm thinking along the lines of wrapping the call to completing-read-function with a condition-case that catches a "completing-read-fallback" signal, and calls completing-read-default instead. So any completion system that wants to punt just has to throw the appropriate signal. Some replies to specific points, which may or may not be moot: > > Please think about providing a different implementation of tmm, which > doesn't use `completing-read'. That will let us know whether > `completing-read' is used only as an implementation detail. > Magit implements a very similar system of using single-key shortcuts to activate items from a textual menu, with multiple levels of menus. It doesn't use completing-read. Of course it does completion. Erase chars in the minibuffer from the right > and then hit TAB - completion. It's not a completion to write home about, > but it's completion. > I would say that the fact that tmm does completion on hitting TAB is an internal implementation detail leaking out, i.e. an unintended side-effect of the fact that it uses completing-read internally. The main functionality of tmm is single-key shortcut access to menu items, not completion of user-entered text with matching strings from a set of choices. > > So pluggable completion backends don't make any sense for tmm, > > > > That too sounds overly broad. I think that you have too readily convinced > yourself that this and that don't make *any possible* sense, and you are > prone to nail things down to prevent what you see as the nonsensical. If > such were truly nonsensical then they would likely never exist, and there > would be no reason to forbid them. > > > > I sense that you think of "pluggable completion backends", that is, uses > of `completing-read-function', as necessarily something similar to what > your code does. > I don't think I'm making a broad statement. My claim is that settings that affect how completion works should not affect tmm because tmm isn't doing completion (or rather, tmm is doing completion only incidentally due to a leaky abstraction). I still stand by this claim, regardless of whether tmm's use of completing-read is considered typical or not. (If you accept this claim, there's probably some other completion-related settings that tmm should also ignore.) [FWIW I don't think of `completing-read-function' as providing only a > "pluggable completion backend". Do you think of > `isearch-search-fun-function' as a "pluggable search backend"? Backend? > frontend? There are many angles to something like completion or search > or... There is not just a backend and a frontend. Likewise, what you call > an "alternative completion system". > Well, I'm not sure exactly what to call it. I suppose "alternative values of completing-read-function" is more technically correct, but it's a real mouthful. Really, we're just talking about a completion function - a value for > `completing-read-function'. That has wide scope - there are few constraints > on what it *must* do. I don't see the justice in adding a constraint for > tmm that it cannot do anything except `completing-read-default'.] > > > > and I can't imagine any other value of completing-read-function that would > make sense for tmm besides completing-read-default. > > > > All you need to imagine is something similar to `completing-read-default' > but slightly different in some respect. That's all you *need* to imagine, > but nothing prevents more imagination.' > I was implicitly assuming that no one would bother to implement a new completion function from scratch unless it differed substantially from an existing one. > > And what you or I can imagine is not really the point. The question is > whether tmm.el must limit itself to `completing-read-default'. I don't see > why it should, just because we know of some values of > `completing-read-function' that don't do the right thing for tmm. > > > > Looking at the "git blame" output for tmm, that call to completing-read > has definitely not been updated since completing-read-function was > introduced except for minor bugfixes, > > > > That's irrelevant. > > > > so it makes sense that tmm would be expecting one and only one > implementation of completing-read. > > > > That does not follow at all. > > > > This kind of argument could (inappropriately, IMO) be > applied to any number of completely normal uses of > `completing-read'. > > I see no reason to impose a dichotomy of either a > `completing-read-function' similar to yours or else > `completing-read-default'. There are likely other > benign values of the variable, besides just > `completing-read-default'. > > > > I'm not trying to set a general precedent here. tmm is the only code that > I'm aware of that uses completing-read in this way. > > > > I agree that it is not a common way of using it. It doesn't follow that > the only possible value of `completing-read-function' that is compatible > with tmm is `completing-read-default'. Not at all. > > > > It sounds like (and no, I haven't looked into it; > it just sounds like it) you have some particular > `completing-read-function' values in mind, which > you have found are incompatible with tmm's use of > `completing-read'. > > > > The alternative completing-read-function providers that I am aware of are > of are ido-ubiquitous (written by me), ivy, and helm. I'll also include > icicles, even though uses some other mechanism besides modifying > completing-read-function. ido-ubiquitous and ivy both have code to > deactivate themselves when completing-read is called by tmm because > otherwise their completion systems would break it, while icicles and helm > simply break tmm when enabled. Thus, to my knowledge there is currently no > other completing-read-function that doesn't break tmm (except for those > that make exceptions specifically for tmm). > > > > Again, it is irrelevant that there are uses of `completing-read-function' > that break tmm. And what you or I am aware of, or even what might exist > anywhere today, does not define the scope of possibilities. > > > > [I drink values of the variable `liquid'. Some values, such as strong > sulfuric acid, are quite incompatible with my proper functioning. That > doesn't mean that the only *possible* value or the only *compatible* > value for me is the default value, `water'. > > > > I drink blindly - I don't know what's in the glass. The only requirement > my mouth imposes is that the variable value be a liquid. It is up to > whoever fills my glass to DTRT.] > > > > And if those uses of `completing-read-function' are incompatible with tmm, > and they thus deactivate themselves for tmm commands, that is exactly the > *right* approach, IMO. It is exactly that which I suggested to you > (without knowing that that is what you already do). > > > > [Tmm does work with Icicles, BTW. (But Icicles does not use > `completing-read-function', among other reasons because it wants to work > also with older Emacs versions.)] > > > > If so, that's not an argument for preventing the use of > other values of `completing-read-function' with tmm. > (Clearly the value `completing-read-default' is fine, > for instance.) That's not an argument for tmm to do > something to prevent all use of `completing-read-function'. > > Instead, it's an argument for the code that defines and > uses a particular `completing-read-function' to take > care of the contexts where it makes sense, and to stop > itself from being used in other contexts, where it might > cause trouble. > > Only that code knows the kinds of context where its own > `completing-read-function' makes sense and where it does > not. Code such as tmm should not try to guess what kinds > of trouble different values of `completing-read-function' > might cause. > > I don't think tmm should throw up its hands and say, "Gee, > there might be some values of `completing-read-function' > that are troublesome, so let's just prevent *all* use of > that variable." That doesn't make sense, to me. > > > > Based on my explanation above, that is precisely what I think tmm should > do: avoid using completing-read-function entirely. > > > > I know you think that, but I don't see why. There are surely values of > `completing-read-function' that do not bother tmm. We know of one already: > `completing-read-default'. Why would you suppose that there can be *no* > others? > > > > To look at it another way, tmm was originally written to use > completing-read as an implementation detail, and later the function that > used to be called completing-read was renamed to completing-read-default, > but tmm was never updated to use the new name. This patch rectifies that. > > > > That's completely imagination. `completing-read-default' is not the new > name of what was once `completing-read'. `completing-read' has never used > code like `completing-read-default'. (It has always been written in C.) > > > > What I would say that perhaps you will think goes a bit in your direction > is this: If you can rewrite `tmm.el' so that it has the *same behavior* > without using `completing-read', and the code is at least as simple and > easy to maintain, then I'd say go ahead and do that. > > > > That would support your idea that `completing-read' is only an > implementation detail etc., and the question of `completing-read-function' > would become moot. > I've provided Magit's popups as an example of similar functionality implemented without completing-read. I'm not a user of tmm, I just want to make sure my completion function doesn't break it. > > > If you want additional suggestions, maybe describe just > what the problem is that your completion function causes > for tmm. It's hard to offer suggestions if you only > state that it is incompatible, without going into any > detail. (Not that you must ask for input about this. > But if you would like some then giving more detail might > help.) > > Please use your own judgment (as I said, I don't really > care much about `tmm'), but a priori this sounds like > overkill. > > It sounds a bit like trying to bend Emacs to fit your > `completing-read-function'. I can understand such a > motivation, believe me; I don't ascribe a bad intention > to you. A guess is that you are not sure what to do, > to prevent inappropriate application of your value of > `completing-read-function' in this or that context. > > If you think it causes trouble in some contexts, or it > is not able to handle some contexts properly, then > I'd suggest you consider walling it off from those use > cases. It might take some time to discover which > contexts it causes trouble for, but that's OK - you > could add them as you discover them. Tmm sounds like > a start. > > > The right approach, IMO, is to teach your code when to > use its `completing-read-function' and when not to use > it. Put differently, consider teaching your > `completing-read-function' when and where to hold back > and just punt to the default behavior. > > > > This is exactly how ido-ubiquitous and ivy both currently work: they > essentially have a blacklist of callers for which they revert to standard > completion. tmm is on the blacklist for both packages. > > > > That's great. Problem solved. That's the right approach, IMO. > > > > Certainly, for any alternative completion system > > > > Any? Again with the superlatives. There is more to the world... > > > > there will be cases where it needs to fall back to standard completion. In > my view, the completion system should be able to determine purely based on > the calling context (i.e. its arguments and any relevant dynamically-bound > variables) whether or not it needs to punt. Making this decision based on > the name of the caller instead of the context to make this decision is > admitting that not enough context was provided. I view it as a workaround, > not a desirable design pattern, and someday in the future I hope to be able > to remove the blacklist from ido-ubiquitous. > > In the case of tmm, the best heuristic I can think of would be to inspect > the key bindings of all the letters and numbers. If any of them are locally > rebound in the minibuffer to something other than self-insert-command, then > punt. > > > > That would be silly, IMHO. There can be plenty of uses of > `completing-read' where letter or number keys are bound to commands other > than `self-insert-command'. > > > > [FWIW, though not directly relevant, when Icicle mode is on there are *no* > keys bound to `self-insert-command' in any of the minibuffer completion > keymaps. (But there are keys bound to `icicle-self-insert'.)] > My heuristic was meant specifically for ido-ubiquitous: the heuristic is that if any letters or numbers are bound to something other than self-insert-command, the caller is probably doing something fancy that won't work well with ido completion. In any case, it was merely meant as one example of how a completion function might infer from context whether it should punt without having to identify callers by name. Anyway, regardless of whether this patch is accepted or not, ido-ubiquitous will still need a blacklist either way, since functions like read-file-name that are already covered by normal ido-mode will always be on that blacklist. So I'm not exactly submitting this patch to make my implementation easier. I just thought that having tmm ignore completing-read-function would make it less fragile and mean one less obstacle for anyone implementing a new complting-read-function. [-- Attachment #2: Type: text/html, Size: 35359 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#27193: 25.2; tmm should use completing-read-default 2017-06-02 22:52 ` Ryan Thompson @ 2017-06-03 0:15 ` Drew Adams 0 siblings, 0 replies; 13+ messages in thread From: Drew Adams @ 2017-06-03 0:15 UTC (permalink / raw) To: Ryan Thompson, 27193 > So given your recommendation that alternative completion > functions should "punt" cases they can't handle back to > completing-read-default, along with the high likelihood > that every alternate completion system is going to have > something that it can't handle and needs to punt on, would > you be interested in a patch that implements that punting > mechanism within completing-read itself, so that > ido-ubiqutous, ivy, helm, and others don't all have to > invent their own punting code? I'm thinking along the lines > of wrapping the call to completing-read-function with a > condition-case that catches a "completing-read-fallback" > signal, and calls completing-read-default instead. So any > completion system that wants to punt just has to throw > the appropriate signal. First, I can tell that you have read and thought about what I wrote. Thanks for that. I have not wasted my time, in that case. For the rest, as you can probably guess, I'm not in really in favor of changing `completing-read'. Tmm is something I don't really care about. `completing-read' is something I do care about. A priori, I would prefer that you do whatever it is that you think you need to do to `tmm.el' than to `completing-read' and related code. That said, what you say does not sound objectionable. It might be helpful. But I represent just one opinion, and I haven't really thought about this. If you think such control is something that could be generally helpful then I'd suggest that you bring it up for discussion on emacs-devel@gnu.org. That should help. In terms of implementation, maybe `catch' and `throw' would be a better fit than `condition-case' with a signal handler, but maybe not. And you might have to worry about where handling takes place, in case of recursive completion - dunno. ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#27193: 25.2; tmm should use completing-read-default 2017-06-02 4:50 bug#27193: 25.2; tmm should use completing-read-default Ryan 2017-06-02 15:26 ` Drew Adams @ 2017-06-03 0:02 ` Glenn Morris 1 sibling, 0 replies; 13+ messages in thread From: Glenn Morris @ 2017-06-03 0:02 UTC (permalink / raw) To: 27193-done Version: 26.1 Thanks; applied as b406174. (FYI the patch in your mail seemed mangled.) ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2017-06-03 0:15 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-06-02 4:50 bug#27193: 25.2; tmm should use completing-read-default Ryan 2017-06-02 15:26 ` Drew Adams 2017-06-02 17:37 ` Ryan Thompson 2017-06-02 20:45 ` Ryan Thompson 2017-06-02 21:25 ` Drew Adams 2017-06-02 21:37 ` Dmitry Gutov 2017-06-02 22:19 ` Drew Adams 2017-06-02 22:24 ` Dmitry Gutov 2017-06-02 23:08 ` Ryan Thompson 2017-06-02 23:10 ` Ryan Thompson 2017-06-02 22:52 ` Ryan Thompson 2017-06-03 0:15 ` Drew Adams 2017-06-03 0:02 ` Glenn Morris
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).