GNU bug report logs - #6286
Ruby Mode Missing Syntax

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 6286 in the body.
You can then email your comments to 6286 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Nick Ewing <nick <at> nickewing.net>
To: bug-gnu-emacs <at> gnu.org
Subject: Ruby Mode Missing Syntax
Date: Thu, 27 May 2010 14:48:33 -0700

Ruby mode does not fully support general delimited strings or regular
expressions in many cases.

General delimited strings using parentheses, brackets, or curly braces
are not supported at all by Ruby mode, therefore the following valid
code is not correctly highlighted:

%Q{This is a string}
%w(foo bar)
etc...

This problem is most apparent when there is a single quote or a double
quote within the general delimited string.  In such a case, the quote
starts highlighting everything following it as a string.  Example:

%Q{foo 'bar}

Ruby mode does support general delimited strings while using a single
character delimiter as such:

%Q|This is a string|

However, if there is a quote contained within this syntax, as follows,
the highlighting breaks:

%Q|foo 'bar|

Also, general delimited strings are restricted to one line by Ruby mode,
however Ruby allows them to be multiline.  Therefore the following valid
Ruby code is not highlighted by Ruby mode:

%w|
  element1
  element2
  element3
|

Finally, regular expressions do not work in all cases where they should.
For example, the following Cucumber step definition (using valid Ruby
syntax) will cause the regular expression contained in it to not be
highlighted.

Given /A user is logged in/

It seems that Ruby mode highlights Given as a constant, which is
understandable, but fails to recognize the regular expression as valid
when after a constant.

I attempted to fix some of these problems myself, but was unable.
Hopefully a more experienced emacs user can provide some help.

Thank you!


In GNU Emacs 23.2.1 (x86_64-apple-darwin, NS apple-appkit-1038.29)
 of 2010-05-08 on black.local
Windowing system distributor `Apple', version 10.3.949
configured using `configure  '--host=x86_64-apple-darwin'
'--build=i686-apple-darwin' '--with-ns'
'build_alias=i686-apple-darwin' 'host_alias=x86_64-apple-darwin'
'CC=gcc -mmacosx-version-min=10.5''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: nil
  value of $XMODIFIERS: nil
  locale-coding-system: nil
  default enable-multibyte-characters: t

Major mode: Debugger

Minor modes in effect:
  global-whitespace-mode: t
  autopair-mode: t
  autopair-global-mode: t
  show-paren-mode: t
  ido-everywhere: t
  textmate-mode: t
  global-hl-line-mode: t
  yas/global-mode: t
  nxhtml-menu-mode: t
  tooltip-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-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
<escape> x <help-echo> <down-mouse-1> <mouse-1> <escape>
x <escape> x <up> <up> <down> <backspace> <backspace>
<backspace> <backspace> <backspace> <backspace> <backspace>
<backspace> <backspace> <backspace> <backspace> <backspace>
<backspace> <backspace> <backspace> <backspace> <backspace>
<backspace> <backspace> <backspace> <backspace> <backspace>
<backspace> <backspace> <backspace> <backspace> <backspace>
<backspace> r e p o r t <tab> <return>

Recent messages:
[yas] warning: won't overwrite keybinding "k e y b i n d i n g" for
snippet "module" in `ruby-mode-map'
[yas] warning: won't overwrite keybinding "k e y b i n d i n g" for
snippet "if" in `ruby-mode-map'
[yas] warning: won't overwrite keybinding "k e y b i n d i n g" for
snippet "for" in `ruby-mode-map'
[yas] warning: won't overwrite keybinding "k e y b i n d i n g" for
snippet "do" in `ruby-mode-map'
[yas] warning: won't overwrite keybinding "k e y b i n d i n g" for
snippet "class" in `ruby-mode-map'
Ido mode enabled [2 times]
For information about GNU Emacs and the GNU system, type C-h C-a.
Entering debugger...
goto-history-element: Beginning of history; no preceding item [2 times]
call-interactively: Text is read-only [2 times]

Load-path shadows:
/Users/nick/.emacs.d/vendor/nxhtml/related/php-mode hides
~/.emacs.d/vendor/php-mode
~/.emacs.d/vendor/whitespace hides
/Applications/Emacs.app/Contents/Resources/lisp/whitespace
/Users/nick/.emacs.d/vendor/cedet-1.0pre7/speedbar/speedbar hides
/Applications/Emacs.app/Contents/Resources/lisp/speedbar
/Users/nick/.emacs.d/vendor/cedet-1.0pre7/speedbar/sb-image hides
/Applications/Emacs.app/Contents/Resources/lisp/sb-image
~/.emacs.d/vendor/linum hides
/Applications/Emacs.app/Contents/Resources/lisp/linum
/Users/nick/.emacs.d/vendor/cedet-1.0pre7/common/ezimage hides
/Applications/Emacs.app/Contents/Resources/lisp/ezimage
/Users/nick/.emacs.d/vendor/cedet-1.0pre7/speedbar/dframe hides
/Applications/Emacs.app/Contents/Resources/lisp/dframe
/Users/nick/.emacs.d/elpa/ruby-mode-1.1/ruby-mode hides
/Applications/Emacs.app/Contents/Resources/lisp/progmodes/ruby-mode
/Users/nick/.emacs.d/vendor/cedet-1.0pre7/eieio/eieio hides
/Applications/Emacs.app/Contents/Resources/lisp/emacs-lisp/eieio
/Users/nick/.emacs.d/vendor/cedet-1.0pre7/eieio/eieio-speedbar hides
/Applications/Emacs.app/Contents/Resources/lisp/emacs-lisp/eieio-speedbar
/Users/nick/.emacs.d/vendor/cedet-1.0pre7/eieio/eieio-opt hides
/Applications/Emacs.app/Contents/Resources/lisp/emacs-lisp/eieio-opt
/Users/nick/.emacs.d/vendor/cedet-1.0pre7/eieio/eieio-datadebug hides
/Applications/Emacs.app/Contents/Resources/lisp/emacs-lisp/eieio-datadebug
/Users/nick/.emacs.d/vendor/cedet-1.0pre7/eieio/eieio-custom hides
/Applications/Emacs.app/Contents/Resources/lisp/emacs-lisp/eieio-custom
/Users/nick/.emacs.d/vendor/cedet-1.0pre7/eieio/eieio-comp hides
/Applications/Emacs.app/Contents/Resources/lisp/emacs-lisp/eieio-comp
/Users/nick/.emacs.d/vendor/cedet-1.0pre7/eieio/eieio-base hides
/Applications/Emacs.app/Contents/Resources/lisp/emacs-lisp/eieio-base
/Users/nick/.emacs.d/vendor/cedet-1.0pre7/eieio/chart hides
/Applications/Emacs.app/Contents/Resources/lisp/emacs-lisp/chart
/Users/nick/.emacs.d/vendor/cedet-1.0pre7/srecode/srecode hides
/Applications/Emacs.app/Contents/Resources/lisp/cedet/srecode
/Users/nick/.emacs.d/vendor/cedet-1.0pre7/semantic/semantic hides
/Applications/Emacs.app/Contents/Resources/lisp/cedet/semantic
/Users/nick/.emacs.d/vendor/cedet-1.0pre7/common/pulse hides
/Applications/Emacs.app/Contents/Resources/lisp/cedet/pulse
/Users/nick/.emacs.d/vendor/cedet-1.0pre7/common/mode-local hides
/Applications/Emacs.app/Contents/Resources/lisp/cedet/mode-local
/Users/nick/.emacs.d/vendor/cedet-1.0pre7/common/inversion hides
/Applications/Emacs.app/Contents/Resources/lisp/cedet/inversion
/Users/nick/.emacs.d/vendor/cedet-1.0pre7/ede/ede hides
/Applications/Emacs.app/Contents/Resources/lisp/cedet/ede
/Users/nick/.emacs.d/vendor/cedet-1.0pre7/common/data-debug hides
/Applications/Emacs.app/Contents/Resources/lisp/cedet/data-debug
/Users/nick/.emacs.d/vendor/cedet-1.0pre7/common/cedet hides
/Applications/Emacs.app/Contents/Resources/lisp/cedet/cedet
/Users/nick/.emacs.d/vendor/cedet-1.0pre7/common/cedet-idutils hides
/Applications/Emacs.app/Contents/Resources/lisp/cedet/cedet-idutils
/Users/nick/.emacs.d/vendor/cedet-1.0pre7/common/cedet-global hides
/Applications/Emacs.app/Contents/Resources/lisp/cedet/cedet-global
/Users/nick/.emacs.d/vendor/cedet-1.0pre7/common/cedet-files hides
/Applications/Emacs.app/Contents/Resources/lisp/cedet/cedet-files
/Users/nick/.emacs.d/vendor/cedet-1.0pre7/common/cedet-cscope hides
/Applications/Emacs.app/Contents/Resources/lisp/cedet/cedet-cscope

Features:
(shadow sort mail-extr message ecomplete rfc822 mml mml-sec
password-cache mm-decode mm-bodies mm-encode mailcap mail-parse rfc2231
rfc2047 rfc2045 qp ietf-drums mailabbrev nnheader gnus-util netrc
time-date mm-util mail-prsvr gmm-utils mailheader canlock sha1 hex-util
hashcash mail-utils emacsbug cedet-edebug debug semantic-el
semantic-bovine bovine-debug semantic-debug highlight-parentheses
warnings erlang-start whitespace autopair paren cus-edit cus-start
cus-load linum ido textmate hl-line yasnippet dropdown-list mumamo-fun
mumamo nxhtml-autostart nxhtml-autoload majmodpri rinari easy-mmode jump
inflections findr ruby-compilation which-func imenu pcomplete ansi-color
inf-ruby ruby-mode nxhtml-menu web-autoload nxhtml-base ecb-layout-defs
ecb ecb-symboldef ecb-analyse ecb-compatibility ecb-winman-support
ecb-autogen autoload ecb-tod ecb-cycle ecb-eshell ecb-help ecb-jde
ecb-method-browser hideshow ecb-file-browser ecb-layout compile comint
ecb-create-layout ecb-compilation ecb-speedbar ecb-common-browser
ecb-cedet-wrapper semanticdb-mode semanticdb-find semanticdb-ref
ecb-navigate ecb-mode-line ecb-face tree-buffer ecb-upgrade ecb-util
ring thingatpt cedet cedet-contrib-load cogre-load cogre-srecode
semantic-edit srecode-template-mode srecode-template srecode-template-wy
semantic-wisent wisent srecode-map srecode-dictionary speedbar-load
sb-info ede-load ede-speedbar ede-files ede ede-base ede-auto
eieio-speedbar semantic-ia-sb semantic-analyze semantic-scope
semantic-analyze-fcn semantic-sort semanticdb-el semanticdb
semantic-ctxt semantic-format semantic-util-modes semantic-util semantic
semantic-lex semantic-tag working fame speedbar sb-image ezimage dframe
assoc eieio-custom ede-source eieio-base srecode-load srecode
semantic-load semantic-fw mode-local find-func derived eieio-load
cedet-load cedet-compat eieio byte-opt bytecomp byte-compile inversion
blank-mode-autoloads gist-autoloads rspec-mode-autoloads
ruby-test-mode-autoloads sass-mode-autoloads advice help-fns
advice-preload info wrap-region-autoloads yaml-mode-autoloads package
color-theme edmacro kmacro wid-edit cl cl-19 sendmail regexp-opt
reporter tooltip ediff-hook vc-hooks lisp-float-type mwheel ns-win
easymenu tool-bar dnd fontset image fringe lisp-mode register page
menu-bar rfn-eshadow timer select scroll-bar mldrag 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 loaddefs button
minibuffer faces cus-face files text-properties overlay md5 base64
format env code-pages mule custom widget hashtable-print-readable
backquote make-network-process ns multi-tty emacs)

Message #12 received at 6286 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 6286 <at> debbugs.gnu.org
Subject: Re: General delimited literals in ruby-mode patch
Date: Tue, 24 Apr 2012 11:43:20 -0400

Hi, Dmitry,

> I wrote a patch that fixes all examples except the last one (which is a
> different issue), and also supports nesting delimiters of the same type
> inside the literal: %r(//([^/])*/)

Thank you very much.

> Do I submit the final patch here, or to the Ruby Redmine tracker?

Here is fine, thanks (tho sending it to the bug-report address rather
than to emacs-devel would have been even better in this case).

> As I understand, the upstream version is maintained to be compatible
> with earlier Emacs versions, and I'm using the
> syntax-propertize-function feature here.

The maintainership is sadly unclear, indeed.  If someone could try to
bring the two versions closer (and keep them in sync) that would be very
appreciated.  Maybe it is enough to bring changes from the Ruby code to
the Emacs code and not the other way around, but only if there's a clear
understanding that the Ruby version is "deprecated" and won't see new
development.  Also, I'm not sure the Emacs version includes all the
functionality of the current Ruby code, so there's some work to do.

> Am I trying to be too clever with the syntax table in
> `ruby-syntax-propertize-general-delimiters'?  I figured it's the
> easiest way to avoid reimplementing `scan-lists'.

It doesn't strike me as too clever (actually it looks like the exact
same trick I used in perl-mode).

> In general, if I have an undercooked patch, should I post a message
> here, or just attach it to the related bug?

Attaching it to the bug report is better, in general, unless the patch
ends up attacking a larger problem.

> Why is the case of (fboundp #'syntax-propertize-rules) being nil still
> being handled, by the way?

I tried to preserve backward compatibility, to ease up merging the
two versions.

> This ruby-mode is not compatible with Emacs 23 either way.

That might be: I have not verified that it is indeed
backward compatible, I just tried to avoid consciously
introducing incompatibilities.

> -                       (or (not (eq ?/ c))
> -                           (null (nth 0 (ruby-parse-region (or begin parse-start) (point)))))
> +                       ;; not a regexp or general delimited literal
> +                       (null (nth 0 (ruby-parse-region (or begin parse-start) (point))))

Could you explain this part of your patch?


        Stefan

Message #15 received at 6286 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: 6286 <at> debbugs.gnu.org
Cc: Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: General delimited literals in ruby-mode patch
Date: Tue, 24 Apr 2012 13:09:21 -0400

I've installed your 2 patches, plus the patch below.
Could you take a look at my patch?  Here is what it does:
- Fix up commenting conventions at random places.
- Split large regexp into more manageable chunks.
- During the split I saw that gsub/sub/split/scan were matched (for
  regexp) without regards to what precedes them, so "asub / a + bsub / b"
  was taken for a regexp.
- I found a problem in your approach to handling Cucumber code.
I don't know Ruby and don't use it (I looked up
http://web.njit.edu/all_topics/Prog_Lang_Docs/html/ruby/syntax.html for
help).
BTW, is it really true that "%Q(hello (my) world)" is correct?
That web-page doesn't clearly mention such nesting.


        Stefan


=== modified file 'lisp/ChangeLog'
--- lisp/ChangeLog	2012-04-24 16:00:08 +0000
+++ lisp/ChangeLog	2012-04-24 16:35:47 +0000
@@ -1,3 +1,10 @@
+2012-04-24  Stefan Monnier  <monnier <at> iro.umontreal.ca>
+
+	* progmodes/ruby-mode.el: Simplify last change, and cleanup code.
+	(ruby-syntax-propertize-regexp): Remove.
+	(ruby-syntax-propertize-function): Split regexp into chunks.
+	Match following code directly.
+
 2012-04-24  Dmitry Gutov  <dgutov <at> yandex.ru>
 
 	* progmodes/ruby-mode.el: Handle Cucumber defs (bug#6286).

=== modified file 'lisp/progmodes/ruby-mode.el'
--- lisp/progmodes/ruby-mode.el	2012-04-24 16:00:08 +0000
+++ lisp/progmodes/ruby-mode.el	2012-04-24 16:42:22 +0000
@@ -784,7 +784,7 @@
                       (not (looking-at "[a-z_]"))))
                (and (looking-at ruby-operator-re)
                     (not (ruby-special-char-p))
-                    ;; operator at the end of line
+                    ;; Operator at the end of line.
                     (let ((c (char-after (point))))
                       (and
 ;;                     (or (null begin)
@@ -794,8 +794,9 @@
 ;;                           (not (or (eolp) (looking-at "#")
 ;;                                    (and (eq (car (nth 1 state)) ?{)
 ;;                                         (looking-at "|"))))))
-                       ;; not a regexp or general delimited literal
-                       (null (nth 0 (ruby-parse-region (or begin parse-start) (point))))
+                       ;; Not a regexp or general delimited literal.
+                       (null (nth 0 (ruby-parse-region (or begin parse-start)
+                                                       (point))))
                        (or (not (eq ?| (char-after (point))))
                            (save-excursion
                              (or (eolp) (forward-char -1))
@@ -1110,6 +1111,8 @@
             mlist)))))
 
 (declare-function ruby-syntax-propertize-heredoc "ruby-mode" (limit))
+(declare-function ruby-syntax-general-delimiters-goto-beg "ruby-mode" ())
+(declare-function ruby-syntax-propertize-general-delimiters "ruby-mode" (limit))
 
 (if (eval-when-compile (fboundp #'syntax-propertize-rules))
     ;; New code that works independently from font-lock.
@@ -1121,18 +1124,37 @@
         (ruby-syntax-general-delimiters-goto-beg)
         (funcall
          (syntax-propertize-rules
-          ;; #{ }, #$hoge, #@foo are not comments
+          ;; #{ }, #$hoge, #@foo are not comments.
           ("\\(#\\)[{$@]" (1 "."))
-          ;; $' $" $` .... are variables
-          ;; ?' ?" ?` are ascii codes
+          ;; $' $" $` .... are variables.
+          ;; ?' ?" ?` are ascii codes.
           ("\\([?$]\\)[#\"'`]"
            (1 (unless (save-excursion
                         ;; Not within a string.
                         (nth 3 (syntax-ppss (match-beginning 0))))
                 (string-to-syntax "\\"))))
-          ;; regexps
-          ("\\(^\\|[[=(,~?:;<>]\\|\\(?:^\\|\\s \\)\\(?:if\\|elsif\\|unless\\|while\\|until\\|when\\|and\\|or\\|&&\\|||\\)\\|g?sub!?\\|scan\\|split!?\\)?\\s *\\(/\\)[^/\n\\\\]*\\(?:\\\\.[^/\n\\\\]*\\)*\\(/\\)"
-           (2 (ruby-syntax-propertize-regexp)))
+          ;; Regexps: regexps are distinguished from division either because
+          ;; of the keyword/symbol before them, or because of the code
+          ;; following them.
+          ((concat
+            ;; Special tokens that can't be followed by a division operator.
+            "\\(?:\\(^\\|[[=(,~?:;<>]\\|\\(?:^\\|\\s \\)"
+            (regexp-opt '("if" "elsif" "unless" "while" "until" "when" "and"
+                          "or" "&&" "||"
+                          "gsub" "gsub!" "sub" "sub!" "scan" "split" "split!"))
+            "\\)\\s *\\)?"
+            ;; The regular expression itself.
+            "\\(/\\)[^/\n\\\\]*\\(?:\\\\.[^/\n\\\\]*\\)*\\(/\\)"
+            ;; Special code that cannot follow a division operator.
+            ;; FIXME: Just because the second slash of "/foo/ do bar" can't
+            ;; be a division, doesn't mean it can't *start* a regexp, as in
+            ;; "x = toto/foo; if /do bar/".
+            "\\([imxo]*\\s *\\(?:,\\|\\_<do\\_>\\)\\)?")
+           (2 (when (or (match-beginning 1) (match-beginning 4))
+                (string-to-syntax "\"/")))
+           (3 (if (or (match-beginning 1) (match-beginning 4))
+                  (string-to-syntax "\"/")
+                (goto-char (match-end 2)))))
           ("^=en\\(d\\)\\_>" (1 "!"))
           ("^\\(=\\)begin\\_>" (1 "!"))
           ;; Handle here documents.
@@ -1143,21 +1165,6 @@
            (1 (prog1 "|" (ruby-syntax-propertize-general-delimiters end)))))
          (point) end))
 
-      (defun ruby-syntax-propertize-regexp ()
-        (let ((syn (string-to-syntax "\"/")))
-          (goto-char (match-end 3))
-          (if (or
-               ;; after paren, comma, operator, control flow keyword,
-               ;; or a method from hardcoded list
-               (match-beginning 1)
-               ;; followed by comma or block
-               (looking-at "[imxo]*\\s *\\(?:,\\|\\<do\\>\\)"))
-              (progn
-                (put-text-property (1- (point)) (point)
-                                   'syntax-table syn)
-                syn)
-            (goto-char (match-end 2)))))
-
       (defun ruby-syntax-propertize-heredoc (limit)
         (let ((ppss (syntax-ppss))
               (res '()))
@@ -1199,7 +1206,7 @@
                parse-sexp-lookup-properties)
           (ignore-errors
             (if cl
-                (progn  ; paired delimiters
+                (progn  ; Paired delimiters.
                   ;; Delimiter pairs of the same kind can be nested
                   ;; inside the literal, as long as they are balanced.
                   ;; Create syntax table that ignores other characters.
@@ -1210,10 +1217,10 @@
                     (save-restriction
                       (narrow-to-region (point) limit)
                       (forward-list))))  ; skip to the paired character
-              ;; single character delimiter
+              ;; Single character delimiter.
               (re-search-forward (concat "[^\\]\\(?:\\\\\\\\\\)*"
                                          (regexp-quote ops)) limit nil))
-            ;; if we reached here, the closing delimiter was found
+            ;; If we reached here, the closing delimiter was found.
             (put-text-property (1- (point)) (point)
                                'syntax-table (string-to-syntax "|")))))
       )
@@ -1260,7 +1267,7 @@
      (4 (7 . ?/))
      (6 (7 . ?/)))
     ("^=en\\(d\\)\\_>" 1 "!")
-    ;; general delimited string
+    ;; General delimited string.
     ("\\(^\\|[[ \t\n<+(,=]\\)\\(%[xrqQwW]?\\([^<[{(a-zA-Z0-9 \n]\\)[^\n\\\\]*\\(\\\\.[^\n\\\\]*\\)*\\(\\3\\)\\)"
      (3 "\"")
      (5 "\""))

=== modified file 'test/ChangeLog'
--- test/ChangeLog	2012-04-11 03:24:26 +0000
+++ test/ChangeLog	2012-04-24 17:02:20 +0000
@@ -1,3 +1,7 @@
+2012-04-24  Stefan Monnier  <monnier <at> iro.umontreal.ca>
+
+	* indent/ruby.rb: New file, to test new syntax-propertize code.
+
 2012-04-11  Glenn Morris  <rgm <at> gnu.org>
 
 	* automated/vc-bzr.el (vc-bzr-test-faulty-bzr-autoloads): New test.

=== added file 'test/indent/ruby.rb'
--- test/indent/ruby.rb	1970-01-01 00:00:00 +0000
+++ test/indent/ruby.rb	2012-04-24 17:00:30 +0000
@@ -0,0 +1,19 @@
+# Don't mis-match "sub" at the end of words.
+a = asub / aslb + bsub / bslb;
+
+b = %Q{This is a "string"}
+c = %w(foo
+ bar
+ baz)
+d = %!hello!
+
+# A "do" after a slash means that slash is not a division, but it doesn't imply
+# it's a regexp-ender, since it can be a regexp-starter instead!
+x = toto / foo; if /do bar/ then
+                  toto = 1
+                end
+
+# Some Cucumber code:
+Given /toto/ do
+  print "hello"
+end

Message #18 received at 6286 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 6286 <at> debbugs.gnu.org
Subject: Re: General delimited literals in ruby-mode patch
Date: Wed, 25 Apr 2012 03:46:59 +0400

Hi Stefan,

Thanks for the answers.

On 24.04.2012 19:43, Stefan Monnier wrote:
> The maintainership is sadly unclear, indeed.  If someone could try to
> bring the two versions closer (and keep them in sync) that would be very
> appreciated.  Maybe it is enough to bring changes from the Ruby code to
> the Emacs code and not the other way around, but only if there's a clear
> understanding that the Ruby version is "deprecated" and won't see new
> development.  Also, I'm not sure the Emacs version includes all the
> functionality of the current Ruby code, so there's some work to do.

They seem to have a ticket for this, but the comments are all in 
Japanese: http://bugs.ruby-lang.org/issues/5142
And Babelfish doesn't help much.
If nobody beats me to it, I'm going to ask on the mailing list later.

>> This ruby-mode is not compatible with Emacs 23 either way.
>
> That might be: I have not verified that it is indeed
> backward compatible, I just tried to avoid consciously
> introducing incompatibilities.

To be precise, when you load it in 23.3, it complains about prog-mode's 
function definition being void.

I guess that means we don't need to worry about maintaining the "else" 
branch when implementing something that requires `syntax-propertize-rules'?

>> -                       (or (not (eq ?/ c))
>> -                           (null (nth 0 (ruby-parse-region (or begin parse-start) (point)))))
>> +                       ;; not a regexp or general delimited literal
>> +                       (null (nth 0 (ruby-parse-region (or begin parse-start) (point))))
>
> Could you explain this part of your patch?

That's a fix for indentation after percent literals delimited with 
operator characters: %r-abc-. But you seem to have already worked that out.

> BTW, is it really true that "%Q(hello (my) world)" is correct?
> That web-page doesn't clearly mention such nesting.

Yes, it seems to be one of the more obscure features:

irb(main):002:0> %Q(hello [(my]) world)
=> "hello [(my]) world"

It's mentioned here: http://phrogz.net/programmingruby/language.html

(to be continued)

-- Dmitry

Message #21 received at 6286 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 6286 <at> debbugs.gnu.org
Subject: Re: General delimited literals in ruby-mode patch
Date: Wed, 25 Apr 2012 07:03:05 +0400

[Message part 1 (text/plain, inline)]

So, the patch.

On 24.04.2012 21:09, Stefan Monnier wrote:
> Here is what it does:
> - Split large regexp into more manageable chunks.

I didn't think to use match groups this way. Very nice.

> - During the split I saw that gsub/sub/split/scan were matched (for
>    regexp) without regards to what precedes them, so "asub / a + bsub / b"
>    was taken for a regexp.

This fix has uncovered another problem: "gsub", "gsub!", "sub", "sub!", 
"scan", "split", and "split!" are not special tokens, those are all 
methods on class String: http://www.ruby-doc.org/core-1.9.3/String.html

The original author just collected the methods most often used with 
regexps. And now this is broken: "abcdec".split /[be]/

One might argue that this isn't the most important use case, and that 
methods with arity > 1 are covered by the second rule (comma after), but 
5 of these 7 methods can be called with just 1 argument. So that would 
mean backward incompatibility.

> - I found a problem in your approach to handling Cucumber code.

I'm assuming you mean this:

x = toto / foo if /do bar/ =~ "dobar" # shortened version

We can add a constraint that "do" is followed by (optionally) |a, d, c| 
(block arguments), and then EOL, since do ... end syntax isn't usually 
used with one-liner blocks, especially not after a regexp argument.

Or we can revert the change and do it the original way.

I looked into how other editors deal with regular expressions in Ruby. 
Vim is whitespace-sensitive. In the example above, the highlighting 
depends on whether you put space before "foo" (so it highlights one or 
the other regexp-looking expression).

Textmate favors the whitelisting approach, like ruby-mode had pre-patch: 
http://www.ruby-forum.com/topic/170852

It has one benefit in that when you've typed the regexp, it's already 
highlighted, before you type the block keyword. Might feel more natural.

In this approach, we'd move the "hardcoded" list of special method names 
to a variable, so that users might customize it, per project.

What do you think?

And here's a patch for another issue (attached).

-- Dmitry

[0001-ruby-mode-Don-t-propertize-percent-literals-inside-s.patch (text/plain, attachment)]

Message #24 received at 6286 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 6286 <at> debbugs.gnu.org
Subject: Re: General delimited literals in ruby-mode patch
Date: Wed, 25 Apr 2012 10:15:21 -0400

> To be precise, when you load it in 23.3, it complains about prog-mode's
> function definition being void.

Ah, that should be easy to fix.

> I guess that means we don't need to worry about maintaining the "else"
> branch when implementing something that requires `syntax-propertize-rules'?

We don't have to improve that "else branch", no, but we do want to
preserve its functionality.  So, what you did in your patch (move the
old font-lock-keywords pattern to the "else branch") was just right.

>>> -                       (or (not (eq ?/ c))
>>> -                           (null (nth 0 (ruby-parse-region (or begin parse-start) (point)))))
>>> +                       ;; not a regexp or general delimited literal
>>> +                       (null (nth 0 (ruby-parse-region (or begin parse-start) (point))))
>> 
>> Could you explain this part of your patch?
> That's a fix for indentation after percent literals delimited with operator
> characters: %r-abc-.

So could it be refined so as to check for a "%" char?  I.e. instead of
removing the old (not (eq ?/ c)), replace it with (not (memq c '(?/ ?%)))?

> But you seem to have already worked that out.

I just assumed the hunk was related to the rest of the patch ;-)

>> BTW, is it really true that "%Q(hello (my) world)" is correct?
>> That web-page doesn't clearly mention such nesting.
> Yes, it seems to be one of the more obscure features:
> irb(main):002:0> %Q(hello [(my]) world)
> => "hello [(my]) world"
> It's mentioned here: http://phrogz.net/programmingruby/language.html

OK, good, thanks.

>> - During the split I saw that gsub/sub/split/scan were matched (for
>> regexp) without regards to what precedes them, so "asub / a + bsub / b"
>> was taken for a regexp.
> This fix has uncovered another problem: "gsub", "gsub!", "sub", "sub!",
> "scan", "split", and "split!" are not special tokens, those are all methods
> on class String: http://www.ruby-doc.org/core-1.9.3/String.html

Aha!

> The original author just collected the methods most often used with
> regexps.  And now this is broken: "abcdec".split /[be]/

Oops.

> One might argue that this isn't the most important use case, and that
> methods with arity > 1 are covered by the second rule (comma after), but
> 5 of these 7 methods can be called with just 1 argument.  So that would mean
> backward incompatibility.

And as we've seen the "check for a comma or a do-block afterwards" is
not a reliable method.

>> - I found a problem in your approach to handling Cucumber code.

> I'm assuming you mean this:
> x = toto / foo if /do bar/ =~ "dobar" # Shortened version.

Yes.

> We can add a constraint that "do" is followed by (optionally) |a, d, c|
> (block arguments), and then EOL, since do ... end syntax isn't usually used
> with one-liner blocks, especially not after a regexp argument.

But that just reduces the likelihood of a problem without eliminating it:

   x = toto / foo if /do
       bar/ =~ "dobar" # Shortened version.

still has the exact same problem.

> I looked into how other editors deal with regular expressions in Ruby.
> Vim is whitespace-sensitive.  In the example above, the highlighting
> depends on whether you put space before "foo" (so it highlights one or
> the other regexp-looking expression).

That sounds like a bad idea: if the / is a division, it's OK because you
can easily decide to add/remove the space as needed, but if that / is
for a regexp it's not as easy because adding/removing that space changes
the regexp.

Or is it also linked to the presence of a preceding space?  That might
not be so bad, then.  E.g. " / " is division but "/ ", " /", and "/"
is regexp.

> Textmate favors the whitelisting approach, like ruby-mode had pre-patch:
> http://www.ruby-forum.com/topic/170852

They mention a good point: since you can always use %r/.../ to make it
clear you're using a regexp, it's better to err on the side of division
when in doubt.

> It has one benefit in that when you've typed the regexp, it's already
> highlighted, before you type the block keyword.  Might feel more natural.

Yes, it's a nice side-advantage.

> In this approach, we'd move the "hardcoded" list of special method names
> to a variable, so that users might customize it, per project.
> What do you think?

Sounds good.

> And here's a patch for another issue (attached).

Regarding that patch, I think that you should do it differently:
- nitpick: rather than goto-char+syntax-ppss, you can just do
  (syntax-ppss (match-beginning 1)).
- leave the (prog1 "|" (ruby-syntax-propertize-general-delimiters end))
  as is.
- instead change ruby-syntax-propertize-general-delimiters so that it
  first checks syntax-ppss to make sure it's inside
  a general delimiter.  This way, if the %Q appeared within a string
  (or a comment, BTW), you'll handle it right: even if you've put
  a "|" syntax on the character, that syntax has no effect on
  syntax-ppss if it appears within a string/comment.
- Once you've done that, you can get rid of
  ruby-syntax-general-delimiters-goto-beg and call
  ruby-syntax-propertize-general-delimiters instead.  You'll notice that
  ruby-syntax-propertize-heredoc follows that model.


        Stefan

Message #27 received at 6286 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 6286 <at> debbugs.gnu.org
Subject: Re: General delimited literals in ruby-mode patch
Date: Sun, 29 Apr 2012 00:20:24 +0400

[Message part 1 (text/plain, inline)]

On 25.04.2012 22:00, Stefan Monnier wrote:
>>> So could it be refined so as to check for a "%" char?  I.e. instead of
>>> removing the old (not (eq ?/ c)), replace it with (not (memq c '(?/ ?%)))?
>> No, 'c' here is the char at the end of the previous line (see comment above
>> "let"), and the closing delimiter can be any punctuation char.
>
> OK, then I don't understand what the code used to do.  That's OK, I have
> plenty of better things to do anyway ;-)

AFAIU, it was just an optimization. '/' was the only binary operator to 
have double syntactic meaning. If c is not it, we definitely have 
continued expression, and can skip checking parse status.

>>> x = toto / foo if /do
>>> bar/ =~ "dobar" # Shortened version.
>>> still has the exact same problem.
>> Yes.  But let's face it, seeing this code in the wild is not very likely.
>
> You might be right.
>
>> And it's as easy to change as // ->  %r//.
>
> I'm not sure what change you're referring to: how should I change the
> above code to make it unambiguous that the first / is a division rather
> than a regexp?

You would move the whole regexp to the next line, or (if the constraint 
I suggested is in place) replace the line break with "\n":

x == toto / foo if /do\nbar/ =~ "dobar"

Anyway, the second patch does away this this approach.

>>> Or is it also linked to the presence of a preceding space?  That might
>>> not be so bad, then.  E.g. " / " is division but "/ ", " /", and "/"
>>> is regexp.
>> The lack of preceding space can turn a regexp into division, but not the
>> other way around.
>
> So you're saying that " / ", "/", and "/ " are for division, whereas " /"
> is for regexps?

Yes, though not without exceptions. "if / boo /" and "if/boo/" are both 
considered to contain regexps. As well as "/ boo /" when not after a word.
The algorithm is likely more complex, I'm just describing how it works 
with specific examples.

>> That's one more reason to do this just like TextMate, though.
>
> You mean because the problem can be fixed on the user's side without
> changing the Ruby code?  Yes, that's a clear advantage, especially when
> browsing other people's code.

If TextMate bundles are easily editable by user, I don't know about that 
(they might be). I just meant that having the same level of syntax 
support across editors is a good thing. So that if you have resort to 
%r//, it's the same for the other team members. And vice versa.

I'm speaking mostly hypothetically, but the percent literals patch is in 
fact indirect result of my colleague using this syntax in TextMate, with 
unclosed paren inside.

>> I'm not exactly sure why, but percent literal starting within a comment is
>> already (not) propertized as expected.
>
> Not in all cases:
>
>     x #= "tot %q/to"; =
>      /toto;
>
> now add&remove a space on the second line; then do the same on the
> first line.
> The approach I suggested is based on years of experience ;-)

I wasn't arguing against the approach, just making an observation. :)
But it's always good to have a counter-example.

I did something close to what you described (patch 0001), but it didn't 
seem proper to call the same function in two different situations 
(before search and after search when we already have match data), so 
there's still two of them.

[0001-lisp-progmodes-ruby-mode.el-Don-t-propertize-percent.patch (text/plain, attachment)]

[0002-lisp-progmodes-ruby-mode.el-Go-back-to-method-whitel.patch (text/plain, attachment)]

Message #30 received at 6286 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 6286 <at> debbugs.gnu.org
Subject: Re: General delimited literals in ruby-mode patch
Date: Thu, 03 May 2012 09:39:40 +0400

[Message part 1 (text/plain, inline)]

Hi again,

I missed the byte compilation error, arising from the fact that 
`syntax-propertize-rules` evaluates regular expressions at compile time.

Options:
1) Remove customizability (see attached patch).
2) Regexp matches any method, then we check against the whitelist in a 
new function.

-- Dmitry

[0003-lisp-progmodes-ruby-mode.el-Fix-byte-compilation-err.patch (text/plain, attachment)]

Message #33 received at 6286 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 6286 <at> debbugs.gnu.org
Subject: Re: General delimited literals in ruby-mode patch
Date: Tue, 14 Aug 2012 07:56:42 +0400

[Message part 1 (text/plain, inline)]

Here are the 3 previously submitted patches as 2 files, slightly 
reworded, and with proper ChangeLog entries.

I think the first one is quite important, and I've had a user emailing 
me about it.
The second one deals with a fairly narrow edge case, so it's kinda optional.

--Dmitry

[01-6286-percent-literals.diff (text/plain, attachment)]

[02-6286-regexps.diff (text/plain, attachment)]

Message #36 received at 6286 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 6286 <at> debbugs.gnu.org
Subject: Re: General delimited literals in ruby-mode patch
Date: Tue, 14 Aug 2012 08:40:04 -0400

> I think the first one is quite important, and I've had a user emailing me
> about it.

Thanks, installed.

> The second one deals with a fairly narrow edge case, so it's kinda optional.

Thanks, installed.

I kind of remember something about this second case, and remember being
bothered by the fact that ruby-syntax-methods-before-regexp would need
to be adjusted by the user since it can depend on its locale
(especially the Given/Then/When).
I also vaguely remember that this problem was related to the reason why
I added the "look after the regexp" test.

It would probably be better if you could commit those changes yourself.
If you want to do that, please request membership in the "emacs" group
from your savannah account (which you may have to create beforehand).


        Stefan


> diff --git a/lisp/ChangeLog b/lisp/ChangeLog
> index 4f52796..f37c346 100644
> --- a/lisp/ChangeLog
> +++ b/lisp/ChangeLog
> @@ -9,6 +9,9 @@
>  	(ruby-syntax-propertize-percent-literal): Only propertize when not
>  	inside a simple string or comment.  When the literal is unclosed,
>  	leave the text after it unpropertized.
> +	(ruby-syntax-methods-before-regexp): New constant.
> +	(ruby-syntax-propertize-function): Use it to recognize regexps.
> +	Don't look at the text after regexp, just use the whitelist.
 
>  2012-08-13  Andreas Schwab  <schwab <at> linux-m68k.org>
 
> diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
> index 42e1ac7..457c7fe 100644
> --- a/lisp/progmodes/ruby-mode.el
> +++ b/lisp/progmodes/ruby-mode.el
> @@ -1178,7 +1178,13 @@ See `add-log-current-defun-function'."
>        (eval-and-compile
>          (defconst ruby-percent-literal-beg-re
>            "\\(%\\)[qQrswWx]?\\([[:punct:]]\\)"
> -          "Regexp to match the beginning of percent literal."))
> +          "Regexp to match the beginning of percent literal.")
> +
> +        (defconst ruby-syntax-methods-before-regexp
> +          '("gsub" "gsub!" "sub" "sub!" "scan" "split" "split!" "index" "match"
> +            "assert_match" "Given" "Then" "When")
> +          "Methods that can take regexp as the first argument.
> +It will be properly highlighted even when the call omits parens."))
 
>        (defun ruby-syntax-propertize-function (start end)
>          "Syntactic keywords for Ruby mode.  See `syntax-propertize-function'."
> @@ -1196,28 +1202,23 @@ See `add-log-current-defun-function'."
>                          ;; Not within a string.
>                          (nth 3 (syntax-ppss (match-beginning 0))))
>                  (string-to-syntax "\\"))))
> -          ;; Regexps: regexps are distinguished from division either because
> -          ;; of the keyword/symbol before them, or because of the code
> -          ;; following them.
> +          ;; Regexps: regexps are distinguished from division because
> +          ;; of the keyword, symbol, or method name before them.
>            ((concat
>              ;; Special tokens that can't be followed by a division operator.
> -            "\\(?:\\(^\\|[[=(,~?:;<>]\\|\\(?:^\\|\\s \\)"
> +            "\\(^\\|[[=(,~?:;<>]"
> +            ;; Control flow keywords and operators following bol or whitespace.
> +            "\\|\\(?:^\\|\\s \\)"
>              (regexp-opt '("if" "elsif" "unless" "while" "until" "when" "and"
> -                          "or" "&&" "||"
> -                          "gsub" "gsub!" "sub" "sub!" "scan" "split" "split!"))
> -            "\\)\\s *\\)?"
> +                          "or" "not" "&&" "||"))
> +            ;; Method name from the list.
> +            "\\|\\_<"
> +            (regexp-opt ruby-syntax-methods-before-regexp)
> +            "\\)\\s *"
>              ;; The regular expression itself.
> -            "\\(/\\)[^/\n\\\\]*\\(?:\\\\.[^/\n\\\\]*\\)*\\(/\\)"
> -            ;; Special code that cannot follow a division operator.
> -            ;; FIXME: Just because the second slash of "/foo/ do bar" can't
> -            ;; be a division, doesn't mean it can't *start* a regexp, as in
> -            ;; "x = toto/foo; if /do bar/".
> -            "\\([imxo]*\\s *\\(?:,\\|\\_<do\\_>\\)\\)?")
> -           (2 (when (or (match-beginning 1) (match-beginning 4))
> -                (string-to-syntax "\"/")))
> -           (3 (if (or (match-beginning 1) (match-beginning 4))
> -                  (string-to-syntax "\"/")
> -                (goto-char (match-end 2)))))
> +            "\\(/\\)[^/\n\\\\]*\\(?:\\\\.[^/\n\\\\]*\\)*\\(/\\)")
> +           (2 (string-to-syntax "\"/"))
> +           (3 (string-to-syntax "\"/")))
>            ("^=en\\(d\\)\\_>" (1 "!"))
>            ("^\\(=\\)begin\\_>" (1 "!"))
>            ;; Handle here documents.
> diff --git a/test/ChangeLog b/test/ChangeLog
> index 9dbca3d..f1bf458 100644
> --- a/test/ChangeLog
> +++ b/test/ChangeLog
> @@ -1,6 +1,6 @@
>  2012-08-14  Dmitry Gutov  <dgutov <at> yandex.ru>
 
> -	* indent/ruby.rb: New examples.
> +	* indent/ruby.rb: Rearrange examples, add new ones.
 
>  2012-08-12  Dmitry Gutov  <dgutov <at> yandex.ru>
 
> diff --git a/test/indent/ruby.rb b/test/indent/ruby.rb
> index 7a9d123..4f2e9e6 100644
> --- a/test/indent/ruby.rb
> +++ b/test/indent/ruby.rb
> @@ -1,24 +1,25 @@
> -# Don't mis-match "sub" at the end of words.
> -a = asub / aslb + bsub / bslb;
> -
> +# Percent literals.
>  b = %Q{This is a "string"}
> -c = %w(foo
> +c = %w!foo
>   bar
> - baz)
> -d = %!hello!
> + baz!
> +d = %(hello (nested) world)
 
>  # Don't propertize percent literals inside strings.
>  "(%s, %s)" % [123, 456]
 
> -# Nor inside comments.
> +# Or inside comments.
>  x = # "tot %q/to"; =
>  y = 2 / 3
 
> -# A "do" after a slash means that slash is not a division, but it doesn't imply
> -# it's a regexp-ender, since it can be a regexp-starter instead!
> -x = toto / foo; if /do bar/ then
> -                  toto = 1
> -                end
> +# Regexp after whitelisted method.
> +"abc".sub /b/, 'd'
> +
> +# Don't mis-match "sub" at the end of words.
> +a = asub / aslb + bsub / bslb;
> +
> +# Highlight the regexp after "if".
> +x = toto / foo if /do bar/ =~ "dobar"
 
>  # Some Cucumber code:
>  Given /toto/ do

Message #39 received at 6286 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 6286 <at> debbugs.gnu.org
Subject: Re: General delimited literals in ruby-mode patch
Date: Tue, 14 Aug 2012 21:46:48 +0400

On 14.08.2012 16:40, Stefan Monnier wrote:
> I kind of remember something about this second case, and remember being
> bothered by the fact that ruby-syntax-methods-before-regexp would need
> to be adjusted by the user since it can depend on its locale
> (especially the Given/Then/When).

While Cucumber features can be localized, ruby-mode should only be used 
with step definitions [1], and those just contain Ruby code, so the 
Given/Then/When method names would be the same, AFAIK.

A user might want to be able to add a new method to the list, 
theoretically, but paren-less calling convention is mostly used in 
DSL-like code (that means, with methods provided by some framework or 
other), so I say we wait until such feature request actually comes.

> I also vaguely remember that this problem was related to the reason why
> I added the "look after the regexp" test.

That was in the original patch I submitted, mostly because it looked 
like a clever solution. But the conservative approach should work, too: 
the canonical (I think) TextMate bundle uses the whitelist [2] without 
major complaints.

> It would probably be better if you could commit those changes yourself.
> If you want to do that, please request membership in the "emacs" group
> from your savannah account (which you may have to create beforehand).

Requested, thanks.

I think we're finished with this bug, though, unless you'd like to see 
some other changes.

--Dmitry

[1] https://github.com/cucumber/cucumber/wiki/Step-Definitions
[2] 
https://github.com/drnic/ruby-tmbundle/blob/master/Syntaxes/Ruby.plist#L882

Message #44 received at 6286-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 6286-done <at> debbugs.gnu.org
Subject: Re: General delimited literals in ruby-mode patch
Date: Tue, 14 Aug 2012 22:33:55 -0400

> I think we're finished with this bug, though, unless you'd like to see some
> other changes.

Indeed, thanks,


        Stefan

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.