From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: [SUSPECTED SPAM] Re: [Emacs-diffs] scratch/widen-less a4ba846: Replace prog-widen with consolidating widen calls Date: Sat, 09 Dec 2017 12:47:04 +0200 Message-ID: <838tecuqjb.fsf@gnu.org> References: <20171129233237.27462.23351@vcs0.savannah.gnu.org> <5d668ce5-1482-a3d4-c01b-7d996a532567@yandex.ru> <20171130214621.GA22157@ACM> <27985594-3bb4-ce88-8928-2ccfeac13eae@yandex.ru> <20171201154913.GB3840@ACM> <1e542021-e389-cca4-6acd-349efddb2652@yandex.ru> <20171201223529.GG3840@ACM> <4a94ec5c-efdd-50f1-ff4d-277f5f45c2df@yandex.ru> <83lgil1qme.fsf@gnu.org> <83d13x1j2s.fsf@gnu.org> <34abea95-c7f7-e8fa-8407-8c2fd2a4cfe1@yandex.ru> <83y3mkzw1n.fsf@gnu.org> <83mv2zzv7z.fsf@gnu.org> <83o9nexy48.fsf@gnu.org> <83d13uxug5.fsf@gnu.org> <41e3f343-816f-d2db-6575-6ef43d54957f@yandex.ru> Reply-To: Eli Zaretskii NNTP-Posting-Host: blaine.gmane.org X-Trace: blaine.gmane.org 1512816457 30058 195.159.176.226 (9 Dec 2017 10:47:37 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sat, 9 Dec 2017 10:47:37 +0000 (UTC) Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org To: Dmitry Gutov Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sat Dec 09 11:47:29 2017 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 1eNcfI-0007YY-S6 for ged-emacs-devel@m.gmane.org; Sat, 09 Dec 2017 11:47:29 +0100 Original-Received: from localhost ([::1]:40789 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eNcfP-0006Ud-Ss for ged-emacs-devel@m.gmane.org; Sat, 09 Dec 2017 05:47:35 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:41049) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eNcfI-0006UK-3k for emacs-devel@gnu.org; Sat, 09 Dec 2017 05:47:29 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eNcfD-0002o9-Ee for emacs-devel@gnu.org; Sat, 09 Dec 2017 05:47:28 -0500 Original-Received: from fencepost.gnu.org ([2001:4830:134:3::e]:59387) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eNcfD-0002nw-9q; Sat, 09 Dec 2017 05:47:23 -0500 Original-Received: from [176.228.60.248] (port=4302 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1eNcfC-0006bh-L8; Sat, 09 Dec 2017 05:47:23 -0500 In-reply-to: <41e3f343-816f-d2db-6575-6ef43d54957f@yandex.ru> (message from Dmitry Gutov on Wed, 6 Dec 2017 20:41:21 +0200) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2001:4830:134:3::e 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:220815 Archived-At: Sorry for the delay in responding: 3 days of absence took their toll. > Cc: emacs-devel@gnu.org > From: Dmitry Gutov > Date: Wed, 6 Dec 2017 20:41:21 +0200 > > Yes, this code has been in Emacs source tree for two years. But: > > It's only used is one line in antlr-mode.el, which wasn't supposed to > start using it before Emacs 26 was released anyway. But it's easy to change. > > On the flip side, prog-indentation-context is *only* well-supported in > python-mode. Not in css-mode, js-mode, ruby-mode or others. In those, > you still have to combine its binding with narrow-to-region, essentially > obviating the need for the second element in the list. In short: we have > the code, and even Emacs, after the said 2 years, isn't supporting it > properly. > > PREVIOUS-CHUNKS isn't supported anywhere either. And, in that case, > there's no code corresponding to it. Just a few pieces of documentation. I think we are failing to communicate. "Not supported anywhere" is not a reason good enough in my book to remove something. We have there a feature that is a super-set of what you need for the MMM related support, so a much better way forward is to leave that stuff, since it should transparently support your use case, or almost so. Arguing that there's a good reason for removing prog-indentation-context would need to show how keeping it _precludes_ some of what you are trying to do, or at least makes it unreasonably hard. And please keep in mind that 2 years without any additional users is not long enough to prove that this feature is useless. Let's also keep in mind that this feature was reviewed at the time and admitted into Emacs, which means Stefan did think at least back then that it could be useful in practice. We should give people a chance before deciding it is completely useless and worthy of removal. > No, the widen calls are conditioned on whether we want all > indent-line-function calls to start in fully widened state. Which seems > like an obvious improvement. Point is, they won't be allowed to call > 'widen' themselves, so the addition of 'widen' to indent-for-tab-command > and indent-according-to-mode makes them act on the whole buffer in the > simple case. Whether we should unconditionally call 'widen' there is now a subject of a separate discussion. For the purposes of this discussion, the only issue that should matter is whether using 'prog-widen' instead of 'widen' in those places will get in the way of the MMM support. I think it doesn't. > > What I see in the branch is this: > > > > 1) the calls to prog-widen are replaced with calls to widen. > > No, they are not. The calls to prog-widen (or widen) made in indentation > related code are removed. I don't think this distinction is important. I presumed that you are removing those widen calls because you added similar calls on higher levels, and didn't mention those removals. If that's not so, please point to specific parts of the patch where the widen calls are removed without that assumption. > The only place in the diff where prog-widen was replaced with widen, is > where it probably shouldn't have been in the first place (that code is > not inside indent-line-function; only tangentially related). And I'm proposing to leave that prog-widen call intact, because by default it's the same as calling 'widen' in the first place. By keeping the prog-widen call there, we make the change run lower risk of being backward incompatible. > > 2) some calls to widen are added > > In code that later either calls indent-line-function or > beginning-of-defun-function. This is about widening unconditionally, so it's now unrelated to the current discussion. > > 3) prog-indentation-context is removed > > Yup. And I see no reason for removing it, because if not set to any non-nil value, it is harmless. If you can explain why leaving it contradicts support for MMM, please do. > > 4) prog-first-column the function is removed, and its calls replaced > > with accessing the (existing) name-sake variable > > Yes. It's not a hard requirement, but there doesn't seem much benefit in > keeping it a function. And if it's a function, what will it return? I don't think it matters what it will return. What matters is that (a) keeping a function doesn't interfere with the MMM support in any way I could see; (b) keeping a function makes the changes fully backward-compatible; and (c) keeping a function will allow future extensions where the value returned is not trivially taken from a variable. > > For 4), we already agreed that keeping a function is better. > > I did not. Allow me to post a quote from my other message: > > If we keep it a function, do we also have a variable prog-first-column? We need to store the value somewhere, or have a way of recomputing it. Whether that is done by keeping a variable, and whether that variable is called the same as the function, are secondary issues; the primary issue IMO is why remove a function that is already there, and is the documented interface for accessing the value. I'd again prefer keeping the stuff unchanged, if just for the reason that this is how the code was submitted 2 years ago, and this is how it was approved. It's not the first time that we have a variable named the same as a function. But if there are good reason to keep the function, while renaming the variable, I will probably agree to renaming the variable much easier than I'd agree to removing the function. > Then, some consumers might have doubt which ones to use, and might opt > to reference the variable. In that case, we lose all benefits of having > it a function (like making its implementation somehow smarter later), > and the only benefit remaining is backward compatibility of having a > function with that name. > > But! Like with PREV-CHUNKS, prog-first-column (and the later prog-widen) > are supposed to be used by the major modes only. There are no references > to either of these functions in the latest antlr-mode.el, and there > shouldn't be. > > And as long as all the calls to that function are inside Emacs, we're > free to change it however, including turning it into a variable. These all are very weak reasons in my eyes. Keeping the documented interfaces stable and backward-compatible is a much stronger argument, and IMO in this case it easily overcomes the above considerations. > > Since prog-widen already calls widen if prog-indentation-context is > > nil (which it is by default), calling prog-widen without setting up > > prog-indentation-context will just call widen; this magically takes > > care of 1). > > > > For 3), if we leave prog-indentation-context in the code, and also > > allow direct calls to widen in modes which don't want to use the > > context, we are not losing anything, while leaving the option of using > > the context to those modes which will want that. This currently > > cannot be used by MMM (AFAIU), but other features which need embedded > > code, such as ANTLR, could still use it, and even MMM will be able to > > do that if it is extended. > > As I hopefully described, the above does not make sense, unfortunately. You explained that you consider the design and implementation of prog-indentation-context unnecessarily complex, and that a simpler design and implementation would be more elegant, given the current practices in related modes. You have _not_ explained why keeping the prog-indentation-context stuff would prevent us from supporting MMM and similar features as easily as under your proposed changes. If there are such aspects of prog-indentation-context, please point them out, because those are _the_only_ ones which would convince me to remove prog-indentation-context. > > 2) can be taken care of as indicated above, thus avoiding any possible > > effects on existing behaviors when MMM is not active. > > Not sure I understand you here. That's the "should we unconditionally widen" issue, which I now took to a separate thread. It was also my attempt to keep the changes safe enough to land them on the release branch. So we can forget about this part for the purposes of this discussion. Thanks.