From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Bozhidar Batsov Newsgroups: gmane.emacs.devel Subject: Re: [ruby-mode] Private/protected method definition layout in Ruby 2.1 Date: Thu, 16 Jan 2014 12:15:49 +0200 Message-ID: References: <0E94BE5F9874463D9B4C2A039A3C24E9@gmail.com> <87r4884gx6.fsf@yandex.ru> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/alternative; boundary=089e01161d26a9e53d04f013b5bf X-Trace: ger.gmane.org 1389867413 5521 80.91.229.3 (16 Jan 2014 10:16:53 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Thu, 16 Jan 2014 10:16:53 +0000 (UTC) Cc: Stefan Monnier , emacs-devel To: Dmitry Gutov Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Thu Jan 16 11:17:00 2014 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1W3k0Q-00038p-FI for ged-emacs-devel@m.gmane.org; Thu, 16 Jan 2014 11:16:58 +0100 Original-Received: from localhost ([::1]:59617 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W3k0P-0001Cc-UW for ged-emacs-devel@m.gmane.org; Thu, 16 Jan 2014 05:16:57 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:56917) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W3jzM-00087V-Cc for emacs-devel@gnu.org; Thu, 16 Jan 2014 05:15:57 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W3jzK-0005qy-HO for emacs-devel@gnu.org; Thu, 16 Jan 2014 05:15:52 -0500 Original-Received: from mail-oa0-x233.google.com ([2607:f8b0:4003:c02::233]:44172) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W3jzK-0005qt-Bh for emacs-devel@gnu.org; Thu, 16 Jan 2014 05:15:50 -0500 Original-Received: by mail-oa0-f51.google.com with SMTP id h16so2158557oag.10 for ; Thu, 16 Jan 2014 02:15:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=DlwpX5ld6KwhavVyHUJTrJzMnPk2Uz8nxzkw4zMpiAE=; b=hejyb7Zra7agMbv4GnGbmSKHKBqQvqzYvDFIgmQ3CAlgGN47kwu0il9Z8kqY6Xts9N pI0miu0e8yl+ZZROenOFFGjm02gOzT11aQQ91G59E94JKKc1nv2z/1oOLelQiNmtGu6l uUd14EK7nYMLIl7VdpHYBaxmi/08VC8Zn0EwYY9nLx8qQd2BelgVXgt86dwqbkP4jsol hqKpSN2byDd3dKXc5Zo142Tg0EzHnY1y3qbQm+mqBI4ULLI7IXkCl9ZNniaMkXJsK+j/ 19PEr0peuZ5zn2GI5PlGuIldXWEz7SURn7yLoBaEQpyFBkO+k+FTswmwv0IoNLnHo+6E lB2w== X-Received: by 10.60.115.138 with SMTP id jo10mr117680oeb.71.1389867349695; Thu, 16 Jan 2014 02:15:49 -0800 (PST) Original-Received: by 10.76.79.102 with HTTP; Thu, 16 Jan 2014 02:15:49 -0800 (PST) In-Reply-To: <87r4884gx6.fsf@yandex.ru> X-Google-Sender-Auth: BfJ-n5soi1uEgrTETb6LbzqfPMc X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2607:f8b0:4003:c02::233 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 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-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:168521 Archived-At: --089e01161d26a9e53d04f013b5bf Content-Type: text/plain; charset=UTF-8 I'm OK with the patch, but it makes configuration a bit more difficult since users should actually know all the alignable keywords. I guess we can mention `ruby-alignable-keywords' in the docstring of `ruby-align-to-stmt-keywords' and consider this a good enough hint for the users. One problem with the current implementation is that it won't play nice with assignments if you won't to treat them differently: x = def something ala bala end There won't be a way to get this indentations at the same time: private def something ala bala end and x = def something ala bala end I think that it might make sense to support a different def alignment for `def` after `private`, `protected`, `public` regardless of the `ruby-align-to-stmt-keywords' value. What I'm saying is that some people might prefer to align `def` with a statement beginning only with access modifier methods. Of course, it seems unlikely that someone will assign the value of a method def to a variable, but in the future a method definition in MRI, but it makes sense in Rubinius for instance. On 16 January 2014 07:47, Dmitry Gutov wrote: > Bozhidar Batsov writes: > > > recent changes we did to accommodate similar alignment for `if/unless > > ` it seems like a good idea to add a defcustom supporting the first > > style as well. > > Come to think of it, why not reuse the same mechanism and defcustom? > > The patch below seems to work. Comments? > > === modified file 'lisp/progmodes/ruby-mode.el' > --- lisp/progmodes/ruby-mode.el 2014-01-10 16:32:45 +0000 > +++ lisp/progmodes/ruby-mode.el 2014-01-16 05:35:37 +0000 > @@ -226,7 +226,10 @@ > :group 'ruby > :safe 'integerp) > > -(defcustom ruby-align-to-stmt-keywords nil > +(defconst ruby-alignable-keywords '(if while unless until begin case for > def) > + "Keywords that can be used in `ruby-align-to-stmt-keywords'.") > + > +(defcustom ruby-align-to-stmt-keywords '(def) > "Keywords after which we align the expression body to statement. > > When nil, an expression that begins with one these keywords is > @@ -250,17 +253,13 @@ > > Only has effect when `ruby-use-smie' is t. > " > - :type '(choice > + :type `(choice > (const :tag "None" nil) > (const :tag "All" t) > (repeat :tag "User defined" > - (choice (const if) > - (const while) > - (const unless) > - (const until) > - (const begin) > - (const case) > - (const for)))) > + (choice ,@(mapcar > + (lambda (kw) (list 'const kw)) > + ruby-alignable-keywords)))) > :group 'ruby > :safe 'listp > :version "24.4") > @@ -639,7 +638,7 @@ > (smie-indent--hanging-p) > ruby-indent-level)) > (`(:after . ,(or "?" ":")) ruby-indent-level) > - (`(:before . ,(or "if" "while" "unless" "until" "begin" "case" "for")) > + (`(:before . ,(guard (memq (intern token) ruby-alignable-keywords))) > (when (not (ruby--at-indentation-p)) > (if (ruby-smie--indent-to-stmt-p token) > (ruby-smie--indent-to-stmt) > > --089e01161d26a9e53d04f013b5bf Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
I'm OK with the patch, but it makes configuration a bi= t more difficult since users should actually know all the alignable keyword= s. I guess we can mention `ruby-alignable-keywords' in the docstring of `ruby-align-to-stmt-keywords' and consider this a good enough hint f= or the users. One problem with the current implementation is that it won= 9;t play nice with assignments if you won't to treat them differently:<= /div>

x =3D def something
=C2=A0 ala
=C2= =A0 bala
end

There won't be a way to= get this indentations at the same time:

priv= ate def something
=C2=A0 ala
=C2=A0 bala
end

and

x =3D def something
=C2= =A0 =C2=A0 =C2=A0 =C2=A0 ala
=C2=A0 =C2=A0 =C2=A0 =C2=A0 bala
=C2=A0 =C2=A0 =C2=A0 end

I think that it might make sense to support a different def alignment for `= def` after `private`, `protected`, `public` regardless of the `ruby-align-t= o-stmt-keywords' value. What I'm saying is that some people
might prefer to align `def` with a statement beginning only with acces= s modifier methods. Of course, it seems unlikely that someone will assign t= he value of a method def to a variable, but in the future a method definiti= on in MRI, but it makes sense in Rubinius for instance.=C2=A0



On 16 January 2014 07:47, Dmitry Gutov <dgutov@yandex.ru> wrote:
Bozhidar Batsov <bozhidar@batsov.com> writes:

> recent changes we did to accommodate similar alignment for `if/unless<= br> > ` it seems like a good idea to add a defcustom supporting the first > style as well.

Come to think of it, why not reuse the same mechanism and defcustom?<= br>
The patch below seems to work. Comments?

=3D=3D=3D modified file 'lisp/progmodes/ruby-mode.el'
--- lisp/progmodes/ruby-mode.el 2014-01-10 16:32:45 +0000
+++ lisp/progmodes/ruby-mode.el 2014-01-16 05:35:37 +0000
@@ -226,7 +226,10 @@
=C2=A0 =C2=A0:group 'ruby
=C2=A0 =C2=A0:safe 'integerp)

-(defcustom ruby-align-to-stmt-keywords nil
+(defconst ruby-alignable-keywords '(if while unless until begin case f= or def)
+ =C2=A0"Keywords that can be used in `ruby-align-to-stmt-keywords'= ;.")
+
+(defcustom ruby-align-to-stmt-keywords '(def)
=C2=A0 =C2=A0"Keywords after which we align the expression body to sta= tement.

=C2=A0When nil, an expression that begins with one these keywords is
@@ -250,17 +253,13 @@

=C2=A0Only has effect when `ruby-use-smie' is t.
=C2=A0"
- =C2=A0:type '(choice
+ =C2=A0:type `(choice
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(const :tag "None" nil)<= br> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(const :tag "All" t)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(repeat :tag "User defined&qu= ot;
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(choice (co= nst if)
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0(const while)
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0(const unless)
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0(const until)
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0(const begin)
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0(const case)
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0(const for))))
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(choice ,@(= mapcar
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 (lambda (kw) (list 'const kw))
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 ruby-alignable-keywords))))
=C2=A0 =C2=A0:group 'ruby
=C2=A0 =C2=A0:safe 'listp
=C2=A0 =C2=A0:version "24.4")
@@ -639,7 +638,7 @@
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(smie-indent--hanging-p)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0ruby-indent-level))
=C2=A0 =C2=A0 =C2=A0(`(:after . ,(or "?" ":")) ruby-ind= ent-level)
- =C2=A0 =C2=A0(`(:before . ,(or "if" "while" "unl= ess" "until" "begin" "case" "for&qu= ot;))
+ =C2=A0 =C2=A0(`(:before . ,(guard (memq (intern token) ruby-alignable-key= words)))
=C2=A0 =C2=A0 =C2=A0 (when (not (ruby--at-indentation-p))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 (if (ruby-smie--indent-to-stmt-p token)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (ruby-smie--indent-to-stmt)


--089e01161d26a9e53d04f013b5bf--