unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#16182: 24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens
@ 2013-12-18  3:55 Dmitry Gutov
       [not found] ` <handler.16182.B.138733895212351.ack@debbugs.gnu.org>
                   ` (4 more replies)
  0 siblings, 5 replies; 27+ messages in thread
From: Dmitry Gutov @ 2013-12-18  3:55 UTC (permalink / raw)
  To: 16182

Judging by the open source code, it's not overly popular, but there's a
definite tendency to align literals in these kind of examples to the
beginning of the statement, not to the opening paren that contains them
(which is what ruby-mode does now, as long as there is any text after
the opening round paren):

https://github.com/intridea/grape/blob/master/README.md#basic-usage

  Status.create!({
    user: current_user,
    text: params[:status]
  })

https://github.com/intridea/grape/blob/master/lib/grape/endpoint.rb#L79

  methods.each do |method|
    route_set.add_route(self, {
      path_info: route.route_compiled,
      request_method: method,
    }, route_info: route)
  end

The code is the trunk indents the second example like this:

  methods.each do |method|
    route_set.add_route(self, {
                          path_info: route.route_compiled,
                          request_method: method,
                        }, route_info: route)
  end

Which is the preferred style? Should ruby-mode support both?

The former style looks more compact, but it can also make the arguments
following the "de-indented" literal look a bit out of place.





^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#16182: Acknowledgement (24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens)
       [not found] ` <handler.16182.B.138733895212351.ack@debbugs.gnu.org>
@ 2013-12-18  4:05   ` Dmitry Gutov
  2013-12-18 10:31     ` Bozhidar Batsov
  0 siblings, 1 reply; 27+ messages in thread
From: Dmitry Gutov @ 2013-12-18  4:05 UTC (permalink / raw)
  To: Bozhidar Batsov, Steve Purcell, Adam Sokolnicki; +Cc: 16182

http://debbugs.gnu.org/cgi/bugreport.cgi?bug=16182

Comments welcome.





^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#16182: Acknowledgement (24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens)
  2013-12-18  4:05   ` bug#16182: Acknowledgement (24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens) Dmitry Gutov
@ 2013-12-18 10:31     ` Bozhidar Batsov
  2013-12-19  4:35       ` Dmitry Gutov
  0 siblings, 1 reply; 27+ messages in thread
From: Bozhidar Batsov @ 2013-12-18 10:31 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Steve Purcell, 16182, Adam Sokolnicki

[-- Attachment #1: Type: text/plain, Size: 1181 bytes --]

Personally I dislike this style, because you’re basically it obscures the semantics of the method arguments, but I acknowledge that I’ve seen it the wild. For me it would be more beneficial if we supported a variant of the style:

methods.each do |method| route_set.add_route( self, { path_info: route.route_compiled, request_method: method, }, route_info: route ) end
The current trunk indents this all wrong:
methods.each do |method| route_set.add_route( self, { path_info: route.route_compiled, request_method: method, }, route_info: route ) end  

Something that’s not mentioned here, but it’s a bigger problem for the users is probably the fact that we don’t support the following indentation style:

x = if something
  do_something
end


Even if I don’t use and (and the majority of Ruby hackers AFAIK) it’s still fairly popular. Currently this is the only supported option (which is arguably the most popular):

x = if something
      do_something
    end



--  
Cheers,
Bozhidar


On Wednesday, December 18, 2013 at 6:05 AM, Dmitry Gutov wrote:

> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=16182
>  
> Comments welcome.  


[-- Attachment #2: Type: text/html, Size: 2586 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#16182: 24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens
  2013-12-18  3:55 bug#16182: 24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens Dmitry Gutov
       [not found] ` <handler.16182.B.138733895212351.ack@debbugs.gnu.org>
@ 2013-12-18 12:42 ` Stefan Monnier
  2013-12-19  4:57   ` Dmitry Gutov
  2013-12-18 17:57 ` bug#16182: Adam Sokolnicki
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 27+ messages in thread
From: Stefan Monnier @ 2013-12-18 12:42 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 16182

>   Status.create!({
>     user: current_user,
>     text: params[:status]
>   })

This looks fine.

> https://github.com/intridea/grape/blob/master/lib/grape/endpoint.rb#L79

>   methods.each do |method|
>     route_set.add_route(self, {
>       path_info: route.route_compiled,
>       request_method: method,
>     }, route_info: route)
>   end

But this looks confusing to me.  If (as a coder) I wanted to keep the
code "not too deeply indented", I'd use something like:

   methods.each do |method|
     route_set.add_route(
       self,
       { path_info: route.route_compiled,
         request_method: method, },
       route_info: route)
   end

But that's just me (and I haven't checked to see if the current
indenter would get this right).


        Stefan





^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#16182:
  2013-12-18  3:55 bug#16182: 24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens Dmitry Gutov
       [not found] ` <handler.16182.B.138733895212351.ack@debbugs.gnu.org>
  2013-12-18 12:42 ` bug#16182: 24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens Stefan Monnier
@ 2013-12-18 17:57 ` Adam Sokolnicki
  2013-12-19  4:48   ` bug#16182: Dmitry Gutov
  2013-12-19 18:39 ` bug#16182: Adam Doppelt
  2013-12-20 12:44 ` bug#16182: Adam Sokolnicki
  4 siblings, 1 reply; 27+ messages in thread
From: Adam Sokolnicki @ 2013-12-18 17:57 UTC (permalink / raw)
  To: 16182

[-- Attachment #1: Type: text/plain, Size: 394 bytes --]

Personally I like the alignment to the beginning of the statement rather to
the open paren. When I'm breaking line on argument list it's because the
line is too long. With indentation to the opened paren the line stays long
despite breaking the line.

I think this is how vim indents ruby code by default.

I you ask me emacs can only support the indentation to the beginning of the
statement.

[-- Attachment #2: Type: text/html, Size: 474 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#16182: Acknowledgement (24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens)
  2013-12-18 10:31     ` Bozhidar Batsov
@ 2013-12-19  4:35       ` Dmitry Gutov
  2013-12-19  9:08         ` Bozhidar Batsov
  0 siblings, 1 reply; 27+ messages in thread
From: Dmitry Gutov @ 2013-12-19  4:35 UTC (permalink / raw)
  To: Bozhidar Batsov; +Cc: Steve Purcell, 16182, Adam Sokolnicki

Bozhidar Batsov <bozhidar@batsov.com> writes:

> Personally I dislike this style, because you’re basically it obscures
> the semantics of the method arguments, but I acknowledge that I’ve
> seen it the wild.

Ok, that's a -1, then.

> For me it would be more beneficial if we supported a
> variant of the style:
>
> methods.each do |method|
>   route_set.add_route(
>     self, {
>       path_info: route.route_compiled,
>       request_method: method,
>     }, route_info: route
>   )
> end

Should work now, with revision 115602.

> Something that’s not mentioned here, but it’s a bigger problem for the
> users is probably the fact that we don’t support the following
> indentation style:
>
> x = if something
> do_something
> end
>
> Even if I don’t use and (and the majority of Ruby hackers AFAIK) it’s
> still fairly popular.

It's actually easy to do now, but the old engine doesn't support that
either. That should be takes as evidence that Emacs Rubyists don't care
about this variant much (I didn't see it in feature requests either).

So I'd rather wait for a request from someone who actually would use it.
If you'd like to add it now, please propose the name of the user option.





^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#16182:
  2013-12-18 17:57 ` bug#16182: Adam Sokolnicki
@ 2013-12-19  4:48   ` Dmitry Gutov
  0 siblings, 0 replies; 27+ messages in thread
From: Dmitry Gutov @ 2013-12-19  4:48 UTC (permalink / raw)
  To: Adam Sokolnicki; +Cc: 16182

Adam Sokolnicki <adam.sokolnicki@gmail.com> writes:

> Personally I like the alignment to the beginning of the statement
> rather to the open paren.

Let's count this as +1.

> When I'm breaking line on argument list it's
> because the line is too long. With indentation to the opened paren the
> line stays long despite breaking the line.

Like Stefan suggests, if the opened paren is immediately followed by a
newline, the arguments will be indented less (but still indented by
additional two columns, compared to the beginning of the statement).

> I think this is how vim indents ruby code by default.

If that's true, could you point to:

* Some other open source project or two using this style.

* Some tutorial or step-by-step instruction for me to test this
  indentation in Vim. Do I need to install anything apart from the base
  distribution?
  Suppose I have the snippet of code typed out. What do I press next?

> I you ask me emacs can only support the indentation to the beginning
> of the statement.

I don't think it's sufficient, by itself.

Take this example:

current_user.statuses.find(params[:id]).update({
  user: current_user,
  text: params[:status]
})

Suppose `update' accepted a second argument, and we wanted to pass it
here, on the next line after the hash. Which column would it be indented
to? 0?





^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#16182: 24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens
  2013-12-18 12:42 ` bug#16182: 24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens Stefan Monnier
@ 2013-12-19  4:57   ` Dmitry Gutov
  2013-12-19 13:47     ` Stefan Monnier
  0 siblings, 1 reply; 27+ messages in thread
From: Dmitry Gutov @ 2013-12-19  4:57 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 16182

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>>   Status.create!({
>>     user: current_user,
>>     text: params[:status]
>>   })
>
> This looks fine.

This is not hard to support (add some `skip-syntax-forward' before the
relevant `smie-indent--hanging-p' call), but does it make sense to
indent this call one way, and when the first argument does not start
with a hanging paren (like `self', in the example below), do it
differently?

>>   methods.each do |method|
>>     route_set.add_route(self, {
>>       path_info: route.route_compiled,
>>       request_method: method,
>>     }, route_info: route)
>>   end

> But this looks confusing to me.  If (as a coder) I wanted to keep the
> code "not too deeply indented", I'd use something like:
>
>    methods.each do |method|
>      route_set.add_route(
>        self,
>        { path_info: route.route_compiled,
>          request_method: method, },
>        route_info: route)
>    end

I agree it makes sense, but this change could be not entirely obvious to
users not familiar with Emacs indentation engines.

> But that's just me (and I haven't checked to see if the current
> indenter would get this right).

It does now.





^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#16182: Acknowledgement (24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens)
  2013-12-19  4:35       ` Dmitry Gutov
@ 2013-12-19  9:08         ` Bozhidar Batsov
  2013-12-19 12:54           ` Bozhidar Batsov
  2013-12-19 13:47           ` Dmitry Gutov
  0 siblings, 2 replies; 27+ messages in thread
From: Bozhidar Batsov @ 2013-12-19  9:08 UTC (permalink / raw)
  To: 16182; +Cc: Steve Purcell, Adam Sokolnicki

[-- Attachment #1: Type: text/plain, Size: 1858 bytes --]

On Thursday, December 19, 2013 at 6:35 AM, Dmitry Gutov wrote:
> Bozhidar Batsov <bozhidar@batsov.com (mailto:bozhidar@batsov.com)> writes:
>  
> > Personally I dislike this style, because you’re basically it obscures
> > the semantics of the method arguments, but I acknowledge that I’ve
> > seen it the wild.
> >  
>  
>  
> Ok, that's a -1, then.
>  
> > For me it would be more beneficial if we supported a
> > variant of the style:
> >  
> > methods.each do |method|
> > route_set.add_route(
> > self, {
> > path_info: route.route_compiled,
> > request_method: method,
> > }, route_info: route
> > )
> > end
> >  
>  
>  
> Should work now, with revision 115602.
>  
> > Something that’s not mentioned here, but it’s a bigger problem for the
> > users is probably the fact that we don’t support the following
> > indentation style:
> >  
> > x = if something
> > do_something
> > end
> >  
> > Even if I don’t use and (and the majority of Ruby hackers AFAIK) it’s
> > still fairly popular.
> >  
>  
>  
> It's actually easy to do now, but the old engine doesn't support that
> either. That should be takes as evidence that Emacs Rubyists don't care
> about this variant much (I didn't see it in feature requests either).
>  
> So I'd rather wait for a request from someone who actually would use it.
> If you'd like to add it now, please propose the name of the user option.
>  
>  

I recall seeing StackOverflow questions about this in Emacs, but as I neither practice nor promote this style I’m indifferent to supporting it in Emacs. I do, however, support it in RuboCop (https://github.com/bbatsov/rubocop/issues/661). If we decide to have it in Emacs we might use a similar name for the config variable - `ruby-end-alignment’ with two options - ‘keword and ‘assignment.   


[-- Attachment #2: Type: text/html, Size: 2842 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#16182: Acknowledgement (24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens)
  2013-12-19  9:08         ` Bozhidar Batsov
@ 2013-12-19 12:54           ` Bozhidar Batsov
  2013-12-19 17:15             ` Dmitry Gutov
  2013-12-19 13:47           ` Dmitry Gutov
  1 sibling, 1 reply; 27+ messages in thread
From: Bozhidar Batsov @ 2013-12-19 12:54 UTC (permalink / raw)
  To: 16182, Gutov Dmitry; +Cc: Steve Purcell, Adam Sokolnicki

[-- Attachment #1: Type: text/plain, Size: 2634 bytes --]

I can confirm that your method args indentation fix is working. I did, however, notice the following problem after I mentioned the if/unless/case with assignment indentation. Consider the following:

res = method do |x, y|
  something
end


Might make sense to indent those like:

res = method do |x, y|
           something
         end


for consistency with the if/unless/case indentation by default. Also:

res =
  method do |x, y|
    something
  end


is actually indented like this:

res =
  method do |x, y|
  something
end


--  
Cheers,
Bozhidar


On Thursday, December 19, 2013 at 11:08 AM, Bozhidar Batsov wrote:

> On Thursday, December 19, 2013 at 6:35 AM, Dmitry Gutov wrote:
> > Bozhidar Batsov <bozhidar@batsov.com (mailto:bozhidar@batsov.com)> writes:
> >  
> > > Personally I dislike this style, because you’re basically it obscures
> > > the semantics of the method arguments, but I acknowledge that I’ve
> > > seen it the wild.
> > >  
> >  
> >  
> > Ok, that's a -1, then.
> >  
> > > For me it would be more beneficial if we supported a
> > > variant of the style:
> > >  
> > > methods.each do |method|
> > > route_set.add_route(
> > > self, {
> > > path_info: route.route_compiled,
> > > request_method: method,
> > > }, route_info: route
> > > )
> > > end
> > >  
> >  
> >  
> > Should work now, with revision 115602.
> >  
> > > Something that’s not mentioned here, but it’s a bigger problem for the
> > > users is probably the fact that we don’t support the following
> > > indentation style:
> > >  
> > > x = if something
> > > do_something
> > > end
> > >  
> > > Even if I don’t use and (and the majority of Ruby hackers AFAIK) it’s
> > > still fairly popular.
> > >  
> >  
> >  
> > It's actually easy to do now, but the old engine doesn't support that
> > either. That should be takes as evidence that Emacs Rubyists don't care
> > about this variant much (I didn't see it in feature requests either).
> >  
> > So I'd rather wait for a request from someone who actually would use it.
> > If you'd like to add it now, please propose the name of the user option.
> >  
> >  
> >  
>  
> I recall seeing StackOverflow questions about this in Emacs, but as I neither practice nor promote this style I’m indifferent to supporting it in Emacs. I do, however, support it in RuboCop (https://github.com/bbatsov/rubocop/issues/661). If we decide to have it in Emacs we might use a similar name for the config variable - `ruby-end-alignment’ with two options - ‘keword and ‘assignment.   
>  


[-- Attachment #2: Type: text/html, Size: 4608 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#16182: Acknowledgement (24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens)
  2013-12-19  9:08         ` Bozhidar Batsov
  2013-12-19 12:54           ` Bozhidar Batsov
@ 2013-12-19 13:47           ` Dmitry Gutov
  1 sibling, 0 replies; 27+ messages in thread
From: Dmitry Gutov @ 2013-12-19 13:47 UTC (permalink / raw)
  To: Bozhidar Batsov; +Cc: Steve Purcell, 16182, Adam Sokolnicki

Bozhidar Batsov <bozhidar@batsov.com> writes:

> I recall seeing StackOverflow questions about this in Emacs, but as I
> neither practice nor promote this style I’m indifferent to supporting
> it in Emacs. I do, however, support it in RuboCop
> (https://github.com/bbatsov/rubocop/issues/661). If we decide to have
> it in Emacs we might use a similar name for the config variable -
> `ruby-end-alignment’ with two options - ‘keword and ‘assignment.

I like this direction, but since there are only two choices, it should
probably be a boolean variable.





^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#16182: 24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens
  2013-12-19  4:57   ` Dmitry Gutov
@ 2013-12-19 13:47     ` Stefan Monnier
  0 siblings, 0 replies; 27+ messages in thread
From: Stefan Monnier @ 2013-12-19 13:47 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 16182

> I agree it makes sense, but this change could be not entirely obvious to
> users not familiar with Emacs indentation engines.

I didn't get this indentation from SMIE.  I wrote SMIE to understand
this style of indentation.  AFAIK this style of indentation is 100%
standard, especially for open braces in C/C++/...


        Stefan





^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#16182: Acknowledgement (24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens)
  2013-12-19 12:54           ` Bozhidar Batsov
@ 2013-12-19 17:15             ` Dmitry Gutov
  2013-12-19 20:33               ` Bozhidar Batsov
  0 siblings, 1 reply; 27+ messages in thread
From: Dmitry Gutov @ 2013-12-19 17:15 UTC (permalink / raw)
  To: Bozhidar Batsov, 16182; +Cc: Steve Purcell, Adam Sokolnicki

On 19.12.2013 14:54, Bozhidar Batsov wrote:
> I can confirm that your method args indentation fix is working. I did,
> however, notice the following problem after I mentioned the
> if/unless/case with assignment indentation. Consider the following:
>
> res = method do |x, y|
>    something
> end
>
> Might make sense to indent those like:
>
> res = method do |x, y|
>             something
>           end
>
> for consistency with the if/unless/case indentation by default.

It might be more consistent, but I don't see any projects doing that. 
For example, ActiveRecord, Grape, Goliath and Rack don't.

Check out the examples at the top:

https://github.com/rails/rails/blob/master/activerecord/lib/active_record/base.rb

https://github.com/rack/rack/blob/master/lib/rack/builder.rb

It can be an option, though.

---

Come to think of it, do you see the "align end to the keyword" style 
much? I did a grep on my gems directory, and the other style seems to be 
prevalent, especially among the non-core gems:

Pry, RDoc, Minitest, Rubygems, EventMachine, Nokogiri align to keyword.

ActiveRecord, ActiveSupport, other Active* gems, Cucumber, Thor, Grape, 
Excon, WebMock, Faraday align to the beginning of the statement.

RSpec uses both. Yard aligns to keyword after "=", but to statement 
after "||=".

Maybe we even should align to the statement by default, because, you 
know, Rails.

> Also:
>
> res =
>    method do |x, y|
>      something
>    end
>
> is actually indented like this:
>
> res =
>    method do |x, y|
>    something
> end

Guess we can special-case this.





^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#16182:
  2013-12-18  3:55 bug#16182: 24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens Dmitry Gutov
                   ` (2 preceding siblings ...)
  2013-12-18 17:57 ` bug#16182: Adam Sokolnicki
@ 2013-12-19 18:39 ` Adam Doppelt
  2013-12-20 12:44 ` bug#16182: Adam Sokolnicki
  4 siblings, 0 replies; 27+ messages in thread
From: Adam Doppelt @ 2013-12-19 18:39 UTC (permalink / raw)
  To: 16182

Coincidentally, I mailed Dmitry about this very issue today! He
encouraged me to chime in on the bug.

In my neck of the woods it's common to indent to the beginning of the statement:

x = if y
  y + 1
end

x = list.each do |i|
  i
end

x = case y
when 1 then 2
end

@x ||= begin
  expensive_operation
  ...
end

CONSTANT = begin
  expensive_operation
  ...
end

I patched my personal ruby-mode to default to this indenting.
Incidentally, as the resident emacs enthusiast my dotfiles are used by
many emacs rubyists here in Seattle.

I'd love to see this supported in the new ruby-mode. Just my two cents.





^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#16182: Acknowledgement (24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens)
  2013-12-19 17:15             ` Dmitry Gutov
@ 2013-12-19 20:33               ` Bozhidar Batsov
  2013-12-19 20:42                 ` Steve Purcell
  2013-12-20  5:21                 ` Dmitry Gutov
  0 siblings, 2 replies; 27+ messages in thread
From: Bozhidar Batsov @ 2013-12-19 20:33 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Steve Purcell, 16182, Adam Sokolnicki

[-- Attachment #1: Type: text/plain, Size: 2943 bytes --]

On Thursday, December 19, 2013 at 7:15 PM, Dmitry Gutov wrote:
> On 19.12.2013 14:54, Bozhidar Batsov wrote:
> > I can confirm that your method args indentation fix is working. I did,
> > however, notice the following problem after I mentioned the
> > if/unless/case with assignment indentation. Consider the following:
> >  
> > res = method do |x, y|
> > something
> > end
> >  
> > Might make sense to indent those like:
> >  
> > res = method do |x, y|
> > something
> > end
> >  
> > for consistency with the if/unless/case indentation by default.
>  
> It might be more consistent, but I don't see any projects doing that.  
> For example, ActiveRecord, Grape, Goliath and Rack don't.
>  
> Check out the examples at the top:
>  
> https://github.com/rails/rails/blob/master/activerecord/lib/active_record/base.rb
>  
> https://github.com/rack/rack/blob/master/lib/rack/builder.rb
>  
> It can be an option, though.
Part of the reason people are not using a particular style from time to time is simply lack of tool support. :-) I guess more people would have used that style if their editor supported it.
>  
> ---
>  
> Come to think of it, do you see the "align end to the keyword" style  
> much? I did a grep on my gems directory, and the other style seems to be  
> prevalent, especially among the non-core gems:
>  
> Pry, RDoc, Minitest, Rubygems, EventMachine, Nokogiri align to keyword.
>  
> ActiveRecord, ActiveSupport, other Active* gems, Cucumber, Thor, Grape,  
> Excon, WebMock, Faraday align to the beginning of the statement.
>  
> RSpec uses both. Yard aligns to keyword after "=", but to statement  
> after "||=".
>  
> Maybe we even should align to the statement by default, because, you  
> know, Rails.
>  
>  

Well, even though I develop Rails apps for a living I wouldn’t say the style used in the Rails codebase should be considered some gold standard - after all they are outdenting “private/protected” there :-) That said - before I started using programming Ruby in Emacs I aligned to the beginning of the statement, but I stopped because this wasn’t supported in ruby-mode. After using the alignment to keyword style for several years I’ve grown to like it a lot (and it seems others are enjoying it as well http://stackoverflow.com/questions/2925028/how-do-you-assign-a-variable-with-the-result-of-a-if-else-block). I’m perfectly fine with alignment to statement becoming the default (although the change of this default would be fairly visible/disruptive, since as it stands keyword alignment is the only supported style and I guess most Rubyists using Emacs employ it).  
  
>  
> > Also:
> >  
> > res =
> > method do |x, y|
> > something
> > end
> >  
> > is actually indented like this:
> >  
> > res =
> > method do |x, y|
> > something
> > end
> >  
>  
>  
> Guess we can special-case this.
That’d be great.  


[-- Attachment #2: Type: text/html, Size: 4507 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#16182: Acknowledgement (24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens)
  2013-12-19 20:33               ` Bozhidar Batsov
@ 2013-12-19 20:42                 ` Steve Purcell
  2013-12-20  5:21                 ` Dmitry Gutov
  1 sibling, 0 replies; 27+ messages in thread
From: Steve Purcell @ 2013-12-19 20:42 UTC (permalink / raw)
  To: Bozhidar Batsov; +Cc: Adam Sokolnicki, 16182, Dmitry Gutov

[-- Attachment #1: Type: text/plain, Size: 1125 bytes --]

On 19 Dec 2013, at 20:33, Bozhidar Batsov <bozhidar@batsov.com> wrote:
> Well, even though I develop Rails apps for a living I wouldn’t say the style used in the Rails codebase should be considered some gold standard - after all they are outdenting “private/protected” there :-) That said - before I started using programming Ruby in Emacs I aligned to the beginning of the statement, but I stopped because this wasn’t supported in ruby-mode. After using the alignment to keyword style for several years I’ve grown to like it a lot (and it seems others are enjoying it as well http://stackoverflow.com/questions/2925028/how-do-you-assign-a-variable-with-the-result-of-a-if-else-block). I’m perfectly fine with alignment to statement becoming the default (although the change of this default would be fairly visible/disruptive, since as it stands keyword alignment is the only supported style and I guess most Rubyists using Emacs employ it). 


I really wouldn’t advise changing the default alignment in this case. The current default is “right” for many if not most existing users.

But +1 for there being options. :-)

[-- Attachment #2: Type: text/html, Size: 1634 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#16182: Acknowledgement (24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens)
  2013-12-19 20:33               ` Bozhidar Batsov
  2013-12-19 20:42                 ` Steve Purcell
@ 2013-12-20  5:21                 ` Dmitry Gutov
  2013-12-20  9:51                   ` Bozhidar Batsov
  1 sibling, 1 reply; 27+ messages in thread
From: Dmitry Gutov @ 2013-12-20  5:21 UTC (permalink / raw)
  To: Bozhidar Batsov; +Cc: Steve Purcell, 16182, Adam Doppelt, Adam Sokolnicki

On 19.12.2013 22:33, Bozhidar Batsov wrote:
> Part of the reason people are not using a particular style from time to
> time is simply lack of tool support. :-) I guess more people would have
> used that style if their editor supported it.

Maybe so. I'll have to return to the "do" block later, since this kind 
of special handling requires finding the beginning of the method chain 
(in the general case) that the block is passed to. Other keywords are 
simpler.

> Well, even though I develop Rails apps for a living I wouldn’t say the
> style used in the Rails codebase should be considered some gold standard
> - after all they are outdenting “private/protected” there :-)

Yuck indeed. :)

 > That said
> - before I started using programming Ruby in Emacs I aligned to the
> beginning of the statement, but I stopped because this wasn’t supported
> in ruby-mode. After using the alignment to keyword style for several
> years I’ve grown to like it a lot (and it seems others are enjoying it
> as well
> http://stackoverflow.com/questions/2925028/how-do-you-assign-a-variable-with-the-result-of-a-if-else-block).
> I’m perfectly fine with alignment to statement becoming the default
> (although the change of this default would be fairly visible/disruptive,
> since as it stands keyword alignment is the only supported style and I
> guess most Rubyists using Emacs employ it).

Well, since there's not much support for changing the defaults, I've 
reverted the special handling of "begin" that already made its way in, 
and added a user option that would control all applicable keywords: 
`ruby-align-to-stmt-keywords', in revision 115624.

Everyone, please try how it works for you, maybe comment on the name, etc.

The feature freeze is in a couple of days, so we have to get the basics 
right.





^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#16182: Acknowledgement (24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens)
  2013-12-20  5:21                 ` Dmitry Gutov
@ 2013-12-20  9:51                   ` Bozhidar Batsov
  2013-12-20 11:57                     ` Dmitry Gutov
  0 siblings, 1 reply; 27+ messages in thread
From: Bozhidar Batsov @ 2013-12-20  9:51 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Steve Purcell, 16182, Adam Doppelt, Adam Sokolnicki

[-- Attachment #1: Type: text/plain, Size: 2388 bytes --]

On Friday, December 20, 2013 at 7:21 AM, Dmitry Gutov wrote:
> On 19.12.2013 22:33, Bozhidar Batsov wrote:
> > Part of the reason people are not using a particular style from time to
> > time is simply lack of tool support. :-) I guess more people would have
> > used that style if their editor supported it.
> >  
>  
>  
> Maybe so. I'll have to return to the "do" block later, since this kind  
> of special handling requires finding the beginning of the method chain  
> (in the general case) that the block is passed to. Other keywords are  
> simpler.
>  
> > Well, even though I develop Rails apps for a living I wouldn’t say the
> > style used in the Rails codebase should be considered some gold standard
> > - after all they are outdenting “private/protected” there :-)
> >  
>  
>  
> Yuck indeed. :)
>  
> > That said
> > - before I started using programming Ruby in Emacs I aligned to the
> > beginning of the statement, but I stopped because this wasn’t supported
> > in ruby-mode. After using the alignment to keyword style for several
> > years I’ve grown to like it a lot (and it seems others are enjoying it
> > as well
> > http://stackoverflow.com/questions/2925028/how-do-you-assign-a-variable-with-the-result-of-a-if-else-block).
> > I’m perfectly fine with alignment to statement becoming the default
> > (although the change of this default would be fairly visible/disruptive,
> > since as it stands keyword alignment is the only supported style and I
> > guess most Rubyists using Emacs employ it).
> >  
>  
>  
> Well, since there's not much support for changing the defaults, I've  
> reverted the special handling of "begin" that already made its way in,  
> and added a user option that would control all applicable keywords:  
> `ruby-align-to-stmt-keywords', in revision 115624.
>  
>  

Just a small nitpick - everything that returns a value is actually an expression, not a statement.
Maybe `ruby-align-to-expr-keywords’ would be a more appropriate name for the option.  
>  
> Everyone, please try how it works for you, maybe comment on the name, etc.
>  
> The feature freeze is in a couple of days, so we have to get the basics  
> right.
>  
>  


Btw, I noticed this in the indent examples:

zoo
  .lose(
  q, p)


Shouldn’t it be:

zoo
  .lose(
    q, p)



   


[-- Attachment #2: Type: text/html, Size: 3728 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#16182: Acknowledgement (24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens)
  2013-12-20  9:51                   ` Bozhidar Batsov
@ 2013-12-20 11:57                     ` Dmitry Gutov
  2013-12-20 15:46                       ` Bozhidar Batsov
  0 siblings, 1 reply; 27+ messages in thread
From: Dmitry Gutov @ 2013-12-20 11:57 UTC (permalink / raw)
  To: Bozhidar Batsov; +Cc: Steve Purcell, 16182, Adam Doppelt, Adam Sokolnicki

On 20.12.2013 11:51, Bozhidar Batsov wrote:
> Just a small nitpick - everything that returns a value is actually an
> expression, not a statement.

It can be both (see "expression statement"). This way it's not 
ambiguous, because I'm really aligning to the statement: the containing 
expression, which follows the bob or an [implicit] semicolon.

In Rubocop, you've chosen to align to just the parent expression. Maybe 
we should find a realistic example where one would be different from the 
other.

> Maybe `ruby-align-to-expr-keywords’ would be a more appropriate name for
> the option.

I was thinking rather of `ruby-align-to-statement'. A non-functional 
change that may be easier to pronounce.

> Btw, I noticed this in the indent examples:
>
> zoo
>    .lose(
>    q, p)
>
> Shouldn’t it be:
>
> zoo
>    .lose(
>      q, p)

Maybe, but that's harder to do. Basically, we'd want to keep the 
additional indentation when and only when the parent token (.), or any 
one of its siblings (in case of a chained method call) are at indentation.

Checking if the parent is at indentation is easy, but finding its 
siblings - not so much.





^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#16182:
  2013-12-18  3:55 bug#16182: 24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens Dmitry Gutov
                   ` (3 preceding siblings ...)
  2013-12-19 18:39 ` bug#16182: Adam Doppelt
@ 2013-12-20 12:44 ` Adam Sokolnicki
  4 siblings, 0 replies; 27+ messages in thread
From: Adam Sokolnicki @ 2013-12-20 12:44 UTC (permalink / raw)
  To: 16182

[-- Attachment #1: Type: text/plain, Size: 1349 bytes --]

>> When I'm breaking line on argument list it's
>> because the line is too long. With indentation to the opened paren the
>> line stays long despite breaking the line.
>
> Like Stefan suggests, if the opened paren is immediately followed by a
> newline, the arguments will be indented less (but still indented by
> additional two columns, compared to the beginning of the statement).

I did not think about it before. It makes sense.

>> I think this is how vim indents ruby code by default.

> If that's true, could you point to:
>
> * Some other open source project or two using this style.
>
> * Some tutorial or step-by-step instruction for me to test this
>   indentation in Vim. Do I need to install anything apart from the > > base
>   distribution?
>   Suppose I have the snippet of code typed out. What do I press next?

Actually that's not true. I was wrong. Sorry for the confusion.

>> I you ask me emacs can only support the indentation to the beginning
>> of the statement.
>
> I don't think it's sufficient, by itself.
>
> Take this example:
>
> current_user.statuses.find(params[:id]).update({
>   user: current_user,
>   text: params[:status]
> })
>
> Suppose `update' accepted a second argument, and we wanted to pass it
> here, on the next line after the hash. Which column would it be indented
> to? 0?

Yes it's not sufficient.

[-- Attachment #2: Type: text/html, Size: 2464 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#16182: Acknowledgement (24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens)
  2013-12-20 11:57                     ` Dmitry Gutov
@ 2013-12-20 15:46                       ` Bozhidar Batsov
  2013-12-21 15:31                         ` Dmitry Gutov
  0 siblings, 1 reply; 27+ messages in thread
From: Bozhidar Batsov @ 2013-12-20 15:46 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Steve Purcell, 16182, Adam Doppelt, Adam Sokolnicki

[-- Attachment #1: Type: text/plain, Size: 1545 bytes --]

On Friday, December 20, 2013 at 1:57 PM, Dmitry Gutov wrote:
> On 20.12.2013 11:51, Bozhidar Batsov wrote:
> > Just a small nitpick - everything that returns a value is actually an
> > expression, not a statement.
> >  
>  
>  
> It can be both (see "expression statement"). This way it's not  
> ambiguous, because I'm really aligning to the statement: the containing  
> expression, which follows the bob or an [implicit] semicolon.
>  
> In Rubocop, you've chosen to align to just the parent expression. Maybe  
> we should find a realistic example where one would be different from the  
> other.
>  
>  

I don’t quite understand what you mean.  
>  
> > Maybe `ruby-align-to-expr-keywords’ would be a more appropriate name for
> > the option.
> >  
>  
>  
> I was thinking rather of `ruby-align-to-statement'. A non-functional  
> change that may be easier to pronounce.
>  
>  

Sounds reasonable.  
>  
> > Btw, I noticed this in the indent examples:
> >  
> > zoo
> > .lose(
> > q, p)
> >  
> > Shouldn’t it be:
> >  
> > zoo
> > .lose(
> > q, p)
> >  
>  
>  
> Maybe, but that's harder to do. Basically, we'd want to keep the  
> additional indentation when and only when the parent token (.), or any  
> one of its siblings (in case of a chained method call) are at indentation.
>  
> Checking if the parent is at indentation is easy, but finding its  
> siblings - not so much.
>  
>  

I guess this can be ignored for now, since such code is not particularly common.  

[-- Attachment #2: Type: text/html, Size: 2775 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#16182: Acknowledgement (24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens)
  2013-12-20 15:46                       ` Bozhidar Batsov
@ 2013-12-21 15:31                         ` Dmitry Gutov
  2013-12-21 15:38                           ` Steve Purcell
  2013-12-21 19:32                           ` Adam Doppelt
  0 siblings, 2 replies; 27+ messages in thread
From: Dmitry Gutov @ 2013-12-21 15:31 UTC (permalink / raw)
  To: Bozhidar Batsov; +Cc: Steve Purcell, 16182, Adam Doppelt, Adam Sokolnicki

On 20.12.2013 17:46, Bozhidar Batsov wrote:
> On Friday, December 20, 2013 at 1:57 PM, Dmitry Gutov wrote:
>> On 20.12.2013 11:51, Bozhidar Batsov wrote:
>>> Just a small nitpick - everything that returns a value is actually an
>>> expression, not a statement.
>>
>> It can be both (see "expression statement"). This way it's not
>> ambiguous, because I'm really aligning to the statement: the containing
>> expression, which follows the bob or an [implicit] semicolon.
>>
>> In Rubocop, you've chosen to align to just the parent expression. Maybe
>> we should find a realistic example where one would be different from the
>> other.
> I don’t quite understand what you mean.

This example is indented just like Robocop master likes with (AlignWith: 
variable):

b = a = if 3 == 4
       1
     else
       2
     end

puts a
puts b


Someone correct me if I'm wrong, but I suspect that users who like less 
indentation would prefer to have the `if' body and closer to be aligned 
to the beginning of the statement, rather than to `a'.

That's what ruby-mode does now if `if' is in `ruby-align-to-stmt-keywords'.

Another reason to pick this behavior is that "align to parent" is harder 
to implement. SMIE has no AST: we can find the position of the parent 
token (=), but finding the position of `a' will require manual seeking.

And `a' could be more than a plan variable: maybe something like 
`b[a+1]' or `foo[:bar][:qux]`.

`=' is also not the only operator we can handle. Aside from its 
variations (||=, etc), we might want to support `||' and others.
And the left side of `||' can be an arbitrary expression.





^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#16182: Acknowledgement (24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens)
  2013-12-21 15:31                         ` Dmitry Gutov
@ 2013-12-21 15:38                           ` Steve Purcell
  2013-12-21 15:53                             ` Dmitry Gutov
  2013-12-21 19:32                           ` Adam Doppelt
  1 sibling, 1 reply; 27+ messages in thread
From: Steve Purcell @ 2013-12-21 15:38 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Adam Sokolnicki, 16182, Adam Doppelt, Bozhidar Batsov

On 21 Dec 2013, at 15:31, Dmitry Gutov <dgutov@yandex.ru> wrote:

> This example is indented just like Robocop master likes with (AlignWith: variable):
> 
> b = a = if 3 == 4
>      1
>    else
>      2
>    end
> 
> puts a
> puts b
> 
> 
> Someone correct me if I'm wrong, but I suspect that users who like less indentation would prefer to have the `if' body and closer to be aligned to the beginning of the statement, rather than to `a’.


I wouldn’t personally consider as reasonable *any* scheme which placed related “if”, “else” and “end” keywords at differing levels of indentation.

To get less indentation in the above case, one should place the “if” on a new line IMO.

Just my 2c. :-)




^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#16182: Acknowledgement (24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens)
  2013-12-21 15:38                           ` Steve Purcell
@ 2013-12-21 15:53                             ` Dmitry Gutov
  2013-12-21 16:49                               ` Steve Purcell
  0 siblings, 1 reply; 27+ messages in thread
From: Dmitry Gutov @ 2013-12-21 15:53 UTC (permalink / raw)
  To: Steve Purcell; +Cc: Adam Sokolnicki, 16182, Adam Doppelt, Bozhidar Batsov

On 21.12.2013 17:38, Steve Purcell wrote:
> I wouldn’t personally consider as reasonable *any* scheme which placed related “if”, “else” and “end” keywords at differing levels of indentation.

You're not the target audience, then. :) Not to worry, this won't be the 
default.

But this style or all over the Rails codebase. And, personally, I quite 
like this usage of `begin' (basically, one of Adam's examples):

a ||= begin
   perform_long_lookup
end

Indenting `if' and friends similarly might be good for consistency, at 
least.

(Again, this example is indented differently by default in ruby-mode.)





^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#16182: Acknowledgement (24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens)
  2013-12-21 15:53                             ` Dmitry Gutov
@ 2013-12-21 16:49                               ` Steve Purcell
  0 siblings, 0 replies; 27+ messages in thread
From: Steve Purcell @ 2013-12-21 16:49 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Adam Sokolnicki, 16182, Adam Doppelt, Bozhidar Batsov

On 21 Dec 2013, at 15:53, Dmitry Gutov <dgutov@yandex.ru> wrote:

> But this style or all over the Rails codebase. And, personally, I quite like this usage of `begin' (basically, one of Adam's examples):
> 
> a ||= begin
>  perform_long_lookup
> end
> 
> Indenting `if' and friends similarly might be good for consistency, at least.
> 
> (Again, this example is indented differently by default in ruby-mode.)


For some reason, that example doesn’t look quite as ugly to me. Any “rescue” clause would presumably be aligned to match the “end”.

Obviously my tastes are not self-consistent.







^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#16182: Acknowledgement (24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens)
  2013-12-21 15:31                         ` Dmitry Gutov
  2013-12-21 15:38                           ` Steve Purcell
@ 2013-12-21 19:32                           ` Adam Doppelt
  2013-12-22  2:01                             ` Dmitry Gutov
  1 sibling, 1 reply; 27+ messages in thread
From: Adam Doppelt @ 2013-12-21 19:32 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Steve Purcell, 16182, Bozhidar Batsov, Adam Sokolnicki

> This example is indented just like Robocop master likes with (AlignWith: variable):
>
> b = a = if 3 == 4
>       1
>     else
>       2
>     end
>
> Someone correct me if I'm wrong, but I suspect that users who like less indentation would prefer to have the `if' body and closer to be aligned to the beginning of the statement, rather than to `a'.


That's right - ideally it would be formatted like so:

b = a = if 3 == 4
  1
else
  2
end

I think 'ruby-align-to-stmt-keywords' is a perfectly fine name, btw.





^ permalink raw reply	[flat|nested] 27+ messages in thread

* bug#16182: Acknowledgement (24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens)
  2013-12-21 19:32                           ` Adam Doppelt
@ 2013-12-22  2:01                             ` Dmitry Gutov
  0 siblings, 0 replies; 27+ messages in thread
From: Dmitry Gutov @ 2013-12-22  2:01 UTC (permalink / raw)
  To: Adam Doppelt; +Cc: Steve Purcell, 16182-done, Bozhidar Batsov, Adam Sokolnicki

On 21.12.2013 21:32, Adam Doppelt wrote:
> I think 'ruby-align-to-stmt-keywords' is a perfectly fine name, btw.

Ah, okay then. Without an overwhelming support for a rename, the current 
name stays.

Thanks everyone, I'm closing this bug now. It has veered off to the 
side, but for now the answer to the original question is no, with only 
one vote in favor. Let's maybe revisit it again sometime.





^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2013-12-22  2:01 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-18  3:55 bug#16182: 24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens Dmitry Gutov
     [not found] ` <handler.16182.B.138733895212351.ack@debbugs.gnu.org>
2013-12-18  4:05   ` bug#16182: Acknowledgement (24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens) Dmitry Gutov
2013-12-18 10:31     ` Bozhidar Batsov
2013-12-19  4:35       ` Dmitry Gutov
2013-12-19  9:08         ` Bozhidar Batsov
2013-12-19 12:54           ` Bozhidar Batsov
2013-12-19 17:15             ` Dmitry Gutov
2013-12-19 20:33               ` Bozhidar Batsov
2013-12-19 20:42                 ` Steve Purcell
2013-12-20  5:21                 ` Dmitry Gutov
2013-12-20  9:51                   ` Bozhidar Batsov
2013-12-20 11:57                     ` Dmitry Gutov
2013-12-20 15:46                       ` Bozhidar Batsov
2013-12-21 15:31                         ` Dmitry Gutov
2013-12-21 15:38                           ` Steve Purcell
2013-12-21 15:53                             ` Dmitry Gutov
2013-12-21 16:49                               ` Steve Purcell
2013-12-21 19:32                           ` Adam Doppelt
2013-12-22  2:01                             ` Dmitry Gutov
2013-12-19 13:47           ` Dmitry Gutov
2013-12-18 12:42 ` bug#16182: 24.3.50; ruby-mode: Indentation style of multiline literals with hanging open paren inside other parens Stefan Monnier
2013-12-19  4:57   ` Dmitry Gutov
2013-12-19 13:47     ` Stefan Monnier
2013-12-18 17:57 ` bug#16182: Adam Sokolnicki
2013-12-19  4:48   ` bug#16182: Dmitry Gutov
2013-12-19 18:39 ` bug#16182: Adam Doppelt
2013-12-20 12:44 ` bug#16182: Adam Sokolnicki

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).