bug#16811: 24.3.50; [ruby-mode] Implicit hash indentation bug

bug#16811: 24.3.50; [ruby-mode] Implicit hash indentation bug

[Bozhidar Batsov]

This nested hash literal is indented correctly:

{
  'HashSyntax' => {
    'EnforcedStyle'   => 'ruby19',
    'SupportedStyles' => %w(ruby19 hash_rockets)
  },
  'SpaceAroundOperators' => { 'Enabled' => true }
}

This one, however, is not:

method('HashSyntax' => {
                        'EnforcedStyle'   => 'ruby19',
                        'SupportedStyles' => %w(ruby19 hash_rockets)
                      },
       'SpaceAroundOperators' => { 'Enabled' => true })

Should be:

method('HashSyntax' => {
         'EnforcedStyle'   => 'ruby19',
         'SupportedStyles' => %w(ruby19 hash_rockets)
       },
       'SpaceAroundOperators' => { 'Enabled' => true })


The fact that the hash is implicit (lacking {}) seems to mess with the
indentation logic.



In GNU Emacs 24.3.50.1 (x86_64-apple-darwin13.0.0, NS apple-appkit-1265.00)
 of 2014-01-27 on Bozhidars-MacBook-Pro.local
Windowing system distributor `Apple', version 10.3.1265
Configured using:
 `configure --prefix=/usr/local/Cellar/emacs/HEAD --without-dbus
 --enable-locallisppath=/usr/local/share/emacs/site-lisp
 --infodir=/usr/local/Cellar/emacs/HEAD/share/info/emacs
 --without-gnutls --with-ns --disable-ns-self-contained'

Important settings:
  locale-coding-system: utf-8-unix

Major mode: Ruby

Minor modes in effect:
  ruby-tools-mode: t
  inf-ruby-minor-mode: t
  diff-auto-refine-mode: t
  subword-mode: t
  guru-mode: t
  erc-truncate-mode: t
  erc-spelling-mode: t
  erc-list-mode: t
  erc-menu-mode: t
  erc-autojoin-mode: t
  erc-ring-mode: t
  erc-networks-mode: t
  erc-pcomplete-mode: t
  erc-track-mode: t
  erc-match-mode: t
  erc-button-mode: t
  erc-fill-mode: t
  erc-stamp-mode: t
  erc-netsplit-mode: t
  erc-irccontrols-mode: t
  erc-noncommands-mode: t
  erc-move-to-prompt-mode: t
  erc-readonly-mode: t
  global-flycheck-mode: t
  flycheck-mode: t
  which-function-mode: t
  flx-ido-mode: t
  ido-ubiquitous-mode: t
  winner-mode: t
  global-undo-tree-mode: t
  undo-tree-mode: t
  whitespace-mode: t
  global-anzu-mode: t
  anzu-mode: t
  projectile-global-mode: t
  projectile-mode: t
  flyspell-mode: t
  volatile-highlights-mode: t
  global-hl-line-mode: t
  shell-dirtrack-mode: t
  recentf-mode: t
  savehist-mode: t
  show-smartparens-global-mode: t
  show-smartparens-mode: t
  smartparens-mode: t
  global-auto-revert-mode: t
  delete-selection-mode: t
  prelude-global-mode: t
  prelude-mode: t
  tooltip-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  size-indication-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
C-f C-f C-f C-f C-f C-f C-f C-f C-f C-f C-f C-f l <return>
M-p C-a M-( g e t - b u f f e r - p r o c e s s SPC
<return> C-p C-p C-= M-w <down-mouse-1> <mouse-1> C-x
b t e s t . r b C-g s-r t e s t . r b <return> <return>
<return> C-x b r b C-s C-s C-s <return> C-p C-p C-p
C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p
C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p
C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p
C-p C-p C-f C-f C-f C-f C-f C-f C-f C-f C-f C-f C-f
C-f C-f C-f C-f C-f C-f C-f C-f C-f C-f C-f C-f C-f
C-f C-f C-SPC C-n C-n C-n C-n C-n C-e C-p C-e C-b M-w
C-x b <return> <return> <return> C-y C-p C-p C-p C-p
C-p C-n C-n <tab> C-n <tab> C-n C-n <tab> C-p C-p C-p
C-p C-p C-p C-p C-p C-p C-p C-p C-p C-p C-a C-k C-k
C-k C-k C-k C-k C-k C-k C-k C-k C-k C-k C-k C-k { C-d
SPC <backspace> C-j <tab> C-n <tab> C-n <tab> C-n <tab>
C-n <tab> C-e <return> } <return> <return> C-x b <return>
C-p C-p C-p C-p C-a C-SPC C-n C-n C-n C-n C-e M-w C-x
b <return> <return> C-y C-p C-p C-p C-p C-a C-d C-d
C-d C-d C-d C-d C-d C-d C-d C-d C-d C-d C-d C-d C-d
C-d C-d C-d C-d m e t h o d C-n <tab> C-n <tab> C-n
<tab> C-n <tab> C-e C-p C-p C-p C-p C-p C-p C-p C-p
C-p C-p C-p C-p C-p C-a M-x r e p o r t <return>

Recent messages:
Wrote /Users/bozhidar/projects/test.rb
Mark set [2 times]
Indenting region...done
Saving file /Users/bozhidar/projects/test.rb...
Wrote /Users/bozhidar/projects/test.rb
Mark set [2 times]
Indenting region...done
Align the elements of a hash literal if they span more than one line.
Beginning of buffer
Literal {

Load-path shadows:
/Users/bozhidar/.emacs.d/elpa/tabulated-list-20120406.2251/tabulated-list hides /usr/local/Cellar/emacs/HEAD/share/emacs/24.3.50/lisp/emacs-lisp/tabulated-list

Features:
(shadow sort emacsbug ielm jka-compr package-build json-fix json rect
vc-annotate vc vc-dispatcher hippie-exp yaml-mode ace-jump-mode
mail-extr ffap url-parse url-vars css-mode-expansions css-mode scss-mode
flymake ruby-tools inf-ruby ruby-mode-expansions smartparens-ruby
ruby-mode smie markdown-mode noutline outline mule-util eieio-opt
speedbar sb-image ezimage dframe find-dired misearch multi-isearch
magit-key-mode magit view epa derived epg epg-config diff-mode
git-rebase-mode git-commit-mode server log-edit message sendmail rfc822
mml mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047
rfc2045 ietf-drums mailabbrev mail-utils gmm-utils mailheader pcvs-util
add-log vc-git executable lisp-mnt network-stream starttls tls cider
cider-mode cider-repl cider-eldoc clojure-test-mode cider-interaction
arc-mode archive-mode cider-client nrepl-client cider-util ewoc
superword subword clojure-mode-expansions clojure-mode inf-lisp
rainbow-mode color rainbow-delimiters elisp-slime-nav guru-mode
prelude-key-chord key-chord prelude-xml nxml-mode-expansions
html-mode-expansions sgml-mode smartparens-html rng-nxml rng-valid
rng-loc rng-uri rng-parse nxml-parse rng-match rng-dt rng-util rng-pttrn
nxml-ns nxml-mode nxml-outln nxml-rap nxml-util nxml-glyph nxml-enc
xmltok prelude-web prelude-scss prelude-scheme prelude-ruby prelude-perl
prelude-org prelude-js prelude-erc erc-truncate erc-autoaway
erc-spelling erc-notify erc-log erc-list erc-menu erc-join erc-ring
erc-networks erc-pcomplete erc-track erc-match erc-button erc-fill
erc-stamp erc-netsplit erc-goodies erc erc-backend erc-compat
prelude-emacs-lisp prelude-css prelude-common-lisp slime-autoloads
prelude-clojure prelude-lisp prelude-c prelude-programming flycheck
help-mode rx f which-func imenu prelude-ido smex flx-ido flx
ido-ubiquitous warnings ido prelude-osx exec-path-from-shell
prelude-global-keybindings prelude-editor winner undo-tree diff esh-var
esh-io esh-cmd esh-opt esh-ext esh-proc esh-arg eldoc esh-groups eshell
esh-module esh-mode esh-util re-builder whitespace browse-kill-ring
midnight ediff-merg ediff-wind ediff-diff ediff-mult ediff-help
ediff-init ediff-util ediff dired-x dired anzu projectile pkg-info
find-func grep compile s bookmark pp expand-region text-mode-expansions
er-basic-expansions expand-region-core expand-region-custom flyspell
ispell etags volatile-highlights hl-line windmove tramp-cache tramp-sh
tramp tramp-compat auth-source gnus-util mm-util mail-prsvr
password-cache tramp-loaddefs trampver shell pcomplete comint ansi-color
ring format-spec recentf tree-widget wid-edit savehist saveplace
diminish smartparens-config smartparens autorevert filenotify delsel
prelude-mode easy-mmode edmacro kmacro prelude-core epl advice help-fns
dash thingatpt prelude-ui zenburn-theme prelude-packages finder-inf
ack-and-a-half-autoloads diminish-autoloads erlang-autoloads
flx-ido-autoloads eieio byte-opt bytecomp byte-compile cconv eieio-core
grizzl-autoloads key-chord-autoloads logito-autoloads info easymenu
cl-macs gv move-text-autoloads pkg-info-autoloads puppet-mode-autoloads
rainbow-delimiters-autoloads request-autoloads rubocop-autoloads
volatile-highlights-autoloads yaml-mode-autoloads package cl cl-loaddefs
cl-lib time-date tooltip electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel ns-win tool-bar dnd fontset image regexp-opt
fringe tabulated-list newcomment lisp-mode prog-mode register page
menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core frame cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
case-table epa-hook jka-cmpr-hook help simple abbrev minibuffer nadvice
loaddefs button faces cus-face macroexp files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote make-network-process cocoa ns
multi-tty emacs)

bug#16811: 24.3.50; [ruby-mode] Implicit hash indentation bug

[Dmitry Gutov]

Bozhidar Batsov <bozhidar@batsov.com> writes:

> Should be:
>
> method('HashSyntax' => {
>          'EnforcedStyle'   => 'ruby19',
>          'SupportedStyles' => %w(ruby19 hash_rockets)
>        },
>        'SpaceAroundOperators' => { 'Enabled' => true })
>
> The fact that the hash is implicit (lacking {}) seems to mess with the
> indentation logic.

Not really. The corresponding example with explicit hash would be

{'HashSyntax' => {
                  'EnforcedStyle'   => 'ruby19',
                  'SupportedStyles' => %w(ruby19 hash_rockets)
                },
 'SpaceAroundOperators' => {
   'Enabled' => true }}

and it has the same problem.

The problem seems to be that (let (smie--parent) (smie-indent--parent))
called between "=>" and "{" doesn't stop at "=>", but goes straight to
the parent "{".

Not sure how to fix that best with the current grammar. Stefan?

bug#16811: 24.3.50; [ruby-mode] Implicit hash indentation bug

[Stefan Monnier]

> The problem seems to be that (let (smie--parent) (smie-indent--parent))

Which does little more than (smie-backward-sexp 'halfsexp) in this case.

> called between "=>" and "{" doesn't stop at "=>", but goes straight to
> the parent "{".

Which seems correct.

> Not sure how to fix that best with the current grammar.  Stefan?

I haven't followed the ruby-smie code lately, so I'm a bit lost.  But on
the surface I'd say you might want to use (smie-rule-prev-p "=>")?

This said, I don't understand why

   {
     'HashSyntax' => {
       'EnforcedStyle'   => 'ruby19',
       'SupportedStyles' => %w(ruby19 hash_rockets)
     },
     'SpaceAroundOperators' => { 'Enabled' => true }
   }

is indented differently from

   {'HashSyntax' => {
                     'EnforcedStyle'   => 'ruby19',
                     'SupportedStyles' => %w(ruby19 hash_rockets)
                   },
    'SpaceAroundOperators' => {
      'Enabled' => true }}

It seems like "it jumps back to { instead of =>" is not the full explanation.


        Stefan

bug#16811: 24.3.50; [ruby-mode] Implicit hash indentation bug

[Dmitry Gutov]

On 20.02.2014 16:11, Stefan Monnier wrote:
>> The problem seems to be that (let (smie--parent) (smie-indent--parent))
>
> Which does little more than (smie-backward-sexp 'halfsexp) in this case.

Yes.

>> called between "=>" and "{" doesn't stop at "=>", but goes straight to
>> the parent "{".
>
> Which seems correct.

Maybe according to the current grammar. But not if we want expressions 
after `=>' to have consistent additional indentation. Which I'm now 
thinking we probably don't, since otherwise, if it works right,

      {
        'HashSyntax' => {
          'EnforcedStyle'   => 'ruby19',
          'SupportedStyles' => %w(ruby19 hash_rockets)
        },

would turn into

      {
        'HashSyntax' => {
            'EnforcedStyle'   => 'ruby19',
            'SupportedStyles' => %w(ruby19 hash_rockets)
          },

and that doesn't look good. So indenting the left and right sides of 
`=>' to the same level is probably the way to go.

Changed in revision 116534, fixing this bug.

Thanks for the questions!

> This said, I don't understand why
>
>     {
>       'HashSyntax' => {
>         'EnforcedStyle'   => 'ruby19',
>         'SupportedStyles' => %w(ruby19 hash_rockets)
>       },
>       'SpaceAroundOperators' => { 'Enabled' => true }
>     }
>
> is indented differently from
>
>     {'HashSyntax' => {
>                       'EnforcedStyle'   => 'ruby19',
>                       'SupportedStyles' => %w(ruby19 hash_rockets)
>                     },
>      'SpaceAroundOperators' => {
>        'Enabled' => true }}
 >
> It seems like "it jumps back to { instead of =>" is not the full explanation.

That's because we always insert an implicit semicolon after { at eol, 
and that's because it's hard to distinguish between a curly that's 
opening a hash and a curly than opens a curly block.

And we need those after block-opening curlies, otherwise some token on 
the first line of the block might consider the curly as its parent, 
align to it, and block-opening curlies themselves align to the beginning 
of the statement.

So

it("is too!") {
   bar
     .qux
}

turns into

it("is too!") {
   bar
   .qux
}