From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Phil Sainty Newsgroups: gmane.emacs.devel Subject: Re: [Emacs-diffs] scratch/so-long 7273fb2: Add so-long library Date: Sat, 12 Jan 2019 15:20:32 +1300 Message-ID: <16bb3884-c2de-b178-afe8-0b13a8b116a8@orcon.net.nz> References: <20190107065207.21793.53271@vcs0.savannah.gnu.org> <20190107065208.BA36C21736@vcs0.savannah.gnu.org> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Trace: blaine.gmane.org 1547260479 25947 195.159.176.226 (12 Jan 2019 02:34:39 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sat, 12 Jan 2019 02:34:39 +0000 (UTC) User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 To: Stefan Monnier , emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sat Jan 12 03:34:35 2019 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1gi985-0006Zu-48 for ged-emacs-devel@m.gmane.org; Sat, 12 Jan 2019 03:34:33 +0100 Original-Received: from localhost ([127.0.0.1]:38521 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gi9AC-0005kR-1L for ged-emacs-devel@m.gmane.org; Fri, 11 Jan 2019 21:36:44 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:53188) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gi9A0-0005Xn-MM for emacs-devel@gnu.org; Fri, 11 Jan 2019 21:36:34 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gi8ud-00065c-C7 for emacs-devel@gnu.org; Fri, 11 Jan 2019 21:20:40 -0500 Original-Received: from smtp-1.orcon.net.nz ([60.234.4.34]:33859) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1gi8ud-00064t-10 for emacs-devel@gnu.org; Fri, 11 Jan 2019 21:20:39 -0500 Original-Received: from [150.107.172.113] (port=60973 helo=[192.168.20.103]) by smtp-1.orcon.net.nz with esmtpa (Exim 4.86_2) (envelope-from ) id 1gi8uW-000322-FD; Sat, 12 Jan 2019 15:20:32 +1300 In-Reply-To: Content-Language: en-GB X-GeoIP: NZ X-Spam_score: -2.9 X-Spam_score_int: -28 X-Spam_bar: -- X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 60.234.4.34 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:232316 Archived-At: Hi Stefan, Thanks for the feedback. I didn't spot this until after I'd posted my last message to the "Performance degradation from long lines" thread. On 11/01/19 7:07 AM, Stefan Monnier wrote: >> +FIXME: +++ once all documentation has been added. > > No need for this FIXME: the whole point of this system is that you don't > need to do anything special to mark entries as "not yet handled" (any `**` > header without a preceding `---` or `+++` has not been handled yet), so > it's "fail safe". That FIXME was just for me -- I knew that I wanted to add something to the manual, but hadn't done so yet, and didn't want to mark the NEWS item +++ until I'd added that, and I didn't want to forget to mark it +++ once I'd done so :) The current branch has this fixed. >> +;; Installation >> +;; ------------ >> +;; And add the following to your init file to enable the library: >> +;; >> +;; (when (require 'so-long nil :noerror) >> +;; (so-long-enable)) > > I think the above should just be > > (so-long-auto-mode 1) > > and so-long-auto-mode (just my suggestion for a name, feel free to > chose something else) should be an autoloaded global minor mode which > activates so-long-mode. I actually spent some time experimenting with that idea last month, as it seemed like a good idea to me as well, but it got sufficiently awkward that I dropped the notion as being more hassle than it was worth. If I define a global mode, then there's another variable which is ostensibly tracking the 'enabled' state of the functionality, which seemed ugly. I tried using :variable in the mode definition to make that use `so-long-enabled' internally, but then in testing I found that I could de-sync the mode and that variable (I forget how, but possibly by calling `so-long-disable' which I need to keep for backwards-compatibility), and then it just struck me that I was making more work and a bit of a headache for myself in an effort to allow people to add '(global-so-long-mode)' in their init files instead of '(so-long-enable)' and I immediately lost all interest in spending more time on achieving that. `so-long-enable' is autoloaded, so I could drop the test for (require 'so-long nil :noerror) in the Commentary. That's a hold- over from the earlier releases, and it seemed harmless to keep, but I'm happy to remove it, as it's not needed for either 27 or ELPA. > On the design side, I think you should merge `so-long` and > `so-long-mode` into a single function and make that function > a (buffer-local) minor-mode (i.e. not have any major mode, just use > fundamental-mode instead). Making it a minor mode rather than > a major-mode will also make it easier to remember the previous > major-mode without any need for a change-major-mode-hook hack. These changes are too significant, so I don't wish to do any of that. There's backwards-compatibility with the earlier releases to consider (so-long-mode has always been a major mode); and I also see a benefit to retaining a major mode option (I have a use-case for using it as a file-local 'mode' on one project, and I've had cause to suggest the same thing to other users). I did consider turning `so-long' into a `so-long-minor-mode' at one point, but I think it either seemed like a fairly needless change, or possibly the idea of implementing a feature which was designed to mess with minor mode states *as* a minor mode seemed wrong to me. 'M-x so-long' is also shorter :) >> +(defvar so-long-enabled t >> + "Set to nil to prevent `so-long' from being triggered automatically.") > > Loading a file should not change the behavior of Emacs, so the default > should be nil unless/until we decide to preload this package. That var does nothing at all if (so-long-enable) hasn't been called; but offhand I think it's fine to set it nil by default, so I'll do that. > Rather than defadvice, please use advice-add. I know defadvice and I don't know advice-add, so perhaps someone else can write those changes. Such changes will need to work in Emacs 25 at least, and ideally back to 24.3 (but as I can no longer build 24.x on my machine, I've not actually tested the current library on that version myself, so I'm not certain that I've retained compatibility with 24.3 in this release). I have vague recollections that advice-add is very different when it comes to ad-get-arg, so note in particular that the signature to hack-local-variables changed in 26.1. That's not an issue for the advice I've written, but if it would be for nadvice, then that needs to be accounted for in any rewrite. > Other than that, looks great, thanks. Cheers, -Phil