From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: [elpa] master 74818d5: Brief Mode v5.86 release. Date: Fri, 19 Oct 2018 12:47:57 -0400 Message-ID: References: <20181018141131.22680.53035@vcs0.savannah.gnu.org> <20181018141132.6E8BA208EC@vcs0.savannah.gnu.org> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: blaine.gmane.org 1539969435 11840 195.159.176.226 (19 Oct 2018 17:17:15 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Fri, 19 Oct 2018 17:17:15 +0000 (UTC) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux) To: emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Fri Oct 19 19:17:11 2018 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1gDYOc-0002yp-G0 for ged-emacs-devel@m.gmane.org; Fri, 19 Oct 2018 19:17:10 +0200 Original-Received: from localhost ([::1]:51808 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gDYQj-0005pQ-1J for ged-emacs-devel@m.gmane.org; Fri, 19 Oct 2018 13:19:21 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:43994) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gDXwn-0001zq-4B for emacs-devel@gnu.org; Fri, 19 Oct 2018 12:48:27 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gDXwi-00033G-3F for emacs-devel@gnu.org; Fri, 19 Oct 2018 12:48:25 -0400 Original-Received: from [195.159.176.226] (port=57126 helo=blaine.gmane.org) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1gDXwf-00030a-3r for emacs-devel@gnu.org; Fri, 19 Oct 2018 12:48:18 -0400 Original-Received: from list by blaine.gmane.org with local (Exim 4.84_2) (envelope-from ) id 1gDXuQ-00013D-3t for emacs-devel@gnu.org; Fri, 19 Oct 2018 18:45:58 +0200 X-Injected-Via-Gmane: http://gmane.org/ Original-Lines: 136 Original-X-Complaints-To: usenet@blaine.gmane.org Cancel-Lock: sha1:HlBRfFtw68ebfGhziP/E8QgXY8M= X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 195.159.176.226 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:230500 Archived-At: >> (... comments about the bash script 'b') > The purpose of this quick launcher bash script "b" is to allow > non-Emacs users a quick way to launch Brief mode even if one never use > Emacs before, in order to elicit potential new Emacs & Brief users. OK. > same thing. 'b' is not intended to be used for experienced users or > for long; therefore I have to assume users don't know how to install > Emacs packages in the first place. Not sure what you mean exactly, here: `b` is a script that's part of the `brief` GNU ELPA package. So to get this script, they've had to "install" (at the very least download and unpack) that package already. The easiest way to install it is with `M-x package-install RET`, AFAIK which will take care of downloading it, unpacking it in a good spot etc... What other/simpler method of installation are you thinking about? I could see an argument for trying to handle the case where the user downloads brief-NN.tar.gz by hand, unpacks it somewhere and then runs `b`, but to support that case, `b` would have to use $0 to find its brief.el neighbor and I don't see any such code in `b`. I only see you searching through: || find_brief ~/.emacs.d/elpa/brief-${BRIEFVERSION} \ || find_brief ~/.emacs.d/elpa/brief \ || find_brief ~/.emacs.d/brief \ || find_brief ~/bin/elisp/brief \ where - ~/.emacs.d/elpa/brief-${BRIEFVERSION} is the normal place where package.el places the package, but in that case, most likely the package is already properly installed by package.el so we don't need to know where it is because Emacs will already know where to find brief.el. - all the other places seem rather arbitrary (I'd only expect ~/.emacs.d/brief to cover some non-negligible proportion of users, and those are likely to be familiar enough with Emacs to know how to use package.el). >>> + BRIEFVERSION ELPA brief version, default "5.86" >>I'd recommend not to default to any specific version, ... > The launcher could actually become more complicated to perform more > extensive search and get rid of this version, but the launch time > will be increased accordingly and give new users a bad impression > that Emacs is slow, compare to vi, vim or nano, just like the rumors > say. Not sure how this relates to what I wrote. Keeping BRIEFVERSION unset by default should just stop the script from considering ~/.emacs.d/elpa/brief-${BRIEFVERSION}, which will still work anyway if the script falls back to assuming that `brief` was installed via package.el. I don't see why/how it would slow things down. >>> +#!/bin/bash >>Do you actually make use of bash-isms? If so, can we avoid them >>without too much extra work? If not, we should say "/bin/sh". > Yes, quite a few bash specific things. I find it difficult to find those, but I usually find it easy to change the code not to use them, so if you can point them out I can try and see if I can remove this dependency without making the code worse. > Linux systems must have bash > installed so even tcsh/csh/zsh users still able to run it. [ You mean GNU/Linux, I think, because most Android systems are very much using Linux but don't have bash installed. ] My remark was unrelated to the interactive shell used by the users: some GNU/Linux systems can run without bash (using e.g. dash as the /bin/sh). >>> + (setq scroll-step 1 \ >>> + scroll-conservatively 101) \ >>> ... >>Why are these (h)scroll settings here instead of adding them to > It's basically telling new users what Emacs global settings are > affecting those (jumpy scrolling) behavior, without a need to look > into brief.el. Are they likely to dig into the code of `b` to find that? Or look for a b.el file in the package (e.g. if they installed it via package.el they may not even know where to look to find b.el)? Maybe you could advertise those in a different way, e.g. in the README, in some special entry in the menu-bar, or in some help buffer that you could pop up on first-use (or if the init file is empty)? >>> (defcustom brief-fake-region-face 'region ; 'secondary-selection >> ... >>You probably want `:type 'face` here. > I found I need to change that to defface otherwise in Emacs GUI > it won't show up as "face" even if I changed to :type 'face. It's not the same: one declares a face, the other declares a variable whose value is the name of another face. But yes, it's usually better to define a new face than a Custom variable that tells which existing face to use. >>The `:group 'brief` is redundant (it defaults to the last group defined > I was following how the builtin "cua" package defines them, it > keeps all the ":group 'cua" too. Lots and lots of Elisp has those redundant :group because in the distant past they were not redundant. >>> +(defun brief-toggle-auto-backup () >>> + "Toggle auto-backup on or off." >>> + (interactive) >>> + (message "Turn Emacs auto-backup %s" >>> + (if (setq auto-save-default (not auto-save-default)) >>> + "ON" "OFF"))) >> You could use a minor mode, here: >> >> (define-minor-mode brief-auto-backup-mode >> "Whether auto-backup is done" >> :global t >> :variable auto-save-default) > > Wouldn't that become more complicated as I still need to define > a function `brief-toggle-auto-backup' to toggle it? I don't follow: the above definition already defines the `brief-auto-backup-mode` command which toggles. >> Why not (defalias 'brief-open-new-line-next #'open-line) ? > They're different, `brief-open-new-line-next' shouldn't split > current line or it will be no different than the key. Oops, sorry you're right. I use C-o so little that I was confused about what it does. You just reminded me how useless is the default C-o keybinding ;-) Stefan