Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Collapse fails in ruby mode if the file contains single-line class. #36

Open
tofuya opened this issue May 30, 2021 · 6 comments
Open

Collapse fails in ruby mode if the file contains single-line class. #36

tofuya opened this issue May 30, 2021 · 6 comments
Labels
bug Something isn't working

Comments

@tofuya
Copy link

tofuya commented May 30, 2021

Description :octocat:

I get an error when I fold the code in a ruby file that contains a single-line class, module, and method.

As a side note, in Ruby, classes, modules, and methods can each be defined in a single line, as shown below:

class Foo < Bar; end
module Baz; end
def foo;  "hello" end
def bar() "hello" end
def foo(bar) = "hello" # since Ruby3.0

Reproduction guide 🪲

  1. Start Emacs
  2. Open the following ruby file
class Foo < Bar; end
module Baz; end
def foo;  "hello" end
def bar() "hello" end
def baz(v) = "hello"
  1. Run origami-close-all-nodes

Observed behaviour: 👀 💔
(error "Offset is not within the range of the node: beg=1 ...")

Expected behaviour: ❤️ 😄
No error in ruby file contains a single-line-definition

System Info 💻

  • OS: darwin
  • Emacs: 27.1
  • Spacemacs: 0.300.0
  • Spacemacs branch: develop (rev. 37c8669d3)
  • Graphic display: nil
  • Distribution: spacemacs
  • Editing style: vim
  • Completion: helm
  • Layers:
(emacs-lisp helm multiple-cursors treemacs)
  • System configuration features: RSVG IMAGEMAGICK GLIB NOTIFY KQUEUE ACL GNUTLS LIBXML2 ZLIB TOOLKIT_SCROLL_BARS XIM NS MODULES THREADS JSON PDUMPER LCMS2 GMP

Backtrace 🐾

Debugger entered--Lisp error: (error "Offset is not within the range of the node: beg=1 ...")
  signal(error ("Offset is not within the range of the node: beg=1 ..."))
  error("Offset is not within the range of the node: beg=%s..." 1 0 20)
  origami-fold-node(1 0 20 t nil nil)
  #f(compiled-function (beg end offset children) #<bytecode 0x1fedf5cec791>)(1 0 20 nil)
  #f(compiled-function (positions) #<bytecode 0x1fedf5ce467d>)(((#("class" 0 5 (face font-lock-keyword-face fontified t)) . 1) (#("end" 0 3 (face font-lock-keyword-face fontified t)) . 0) (#("module" 0 6 (face font-lock-keyword-face fontified t)) . 22) (#("end" 0 3 (face font-lock-keyword-face fontified t)) . 21) (#("def" 0 3 (face font-lock-keyword-face fontified t)) . 44) (#("end" 0 3 (face font-lock-keyword-face fontified t)) . 43) (#("def" 0 3 (face font-lock-keyword-face fontified t)) . 66) (#("end" 0 2 (face font-lock-keyword-face fontified t) 2 3 (face font-lock-keyword-face fontified t rear-nonsticky t)) . 65) (#("def" 0 3 (face font-lock-keyword-face fontified t)) . 88)))
  origami-build-pair-tree(#f(compiled-function (beg end offset children) #<bytecode 0x1fedf5cec791>) "\\(s*def\\|class\\|module\\|if\\|unless\\|while\\|until\\|..." "\\(s*end\\)\\_>" "\\(s*else\\|when\\|elsif\\)\\_>" ((#("class" 0 5 (face font-lock-keyword-face fontified t)) . 1) (#("end" 0 3 (face font-lock-keyword-face fontified t)) . 0) (#("module" 0 6 (face font-lock-keyword-face fontified t)) . 22) (#("end" 0 3 (face font-lock-keyword-face fontified t)) . 21) (#("def" 0 3 (face font-lock-keyword-face fontified t)) . 44) (#("end" 0 3 (face font-lock-keyword-face fontified t)) . 43) (#("def" 0 3 (face font-lock-keyword-face fontified t)) . 66) (#("end" 0 2 (face font-lock-keyword-face fontified t) 2 3 (face font-lock-keyword-face fontified t rear-nonsticky t)) . 65) (#("def" 0 3 (face font-lock-keyword-face fontified t)) . 88)) #f(compiled-function (&rest _) #<bytecode 0x1fedf4cb7409>))
  #f(compiled-function (content) #<bytecode 0x1fedf5cec7c9>)(#("class Foo < Bar; end\nmodule Foo < Bar; end\ndef foo..." 0 5 (face font-lock-keyword-face fontified t) 5 6 (fontified t) 6 9 (face font-lock-type-face fontified t) 9 12 (fontified t) 12 15 (face font-lock-type-face fontified t) 15 17 (fontified t) 17 20 (face font-lock-keyword-face fontified t) 20 21 (fontified t rear-nonsticky t) 21 27 (face font-lock-keyword-face fontified t) 27 28 (fontified t) 28 31 (face font-lock-type-face fontified t) 31 34 (fontified t) 34 37 (face font-lock-type-face fontified t) 37 39 (fontified t) 39 42 (face font-lock-keyword-face fontified t) 42 43 (fontified t rear-nonsticky t) ...))
  #f(compiled-function (content) #<bytecode 0x1fedf5ce4341>)(#("class Foo < Bar; end\nmodule Foo < Bar; end\ndef foo..." 0 5 (face font-lock-keyword-face fontified t) 5 6 (fontified t) 6 9 (face font-lock-type-face fontified t) 9 12 (fontified t) 12 15 (face font-lock-type-face fontified t) 15 17 (fontified t) 17 20 (face font-lock-keyword-face fontified t) 20 21 (fontified t rear-nonsticky t) 21 27 (face font-lock-keyword-face fontified t) 27 28 (fontified t) 28 31 (face font-lock-type-face fontified t) 31 34 (fontified t) 34 37 (face font-lock-type-face fontified t) 37 39 (fontified t) 39 42 (face font-lock-keyword-face fontified t) 42 43 (fontified t rear-nonsticky t) ...))
  origami-build-tree(#<buffer test.rb> #f(compiled-function (content) #<bytecode 0x1fedf5ce4341>))
  origami-get-fold-tree(#<buffer test.rb>)
  origami-close-all-nodes(#<buffer test.rb>)
  funcall-interactively(origami-close-all-nodes #<buffer test.rb>)
  call-interactively(origami-close-all-nodes nil nil)
  command-execute(origami-close-all-nodes)
@jcs090218
Copy link
Member

jcs090218 commented May 30, 2021

Thanks for the detail bug report!

I think this will require a rewrite to the ruby core parser since the current logic doesn't support single line folding and the parser will just assume the end of line will be the start of the folding region.

def foo;  # Folding start here
  "hello" 
end

in single line

def foo;  "hello" end  # Folding overlaps (to the end of line)

The logic needs to be updated so we can parser the ruby grammar better. Unlike C-like languages, it does not have simple way to find the proper folding region.


P.S. Any similar languages will have the same issue. Ruby, Lua, BASIC, etc.

@jcs090218 jcs090218 added the bug Something isn't working label May 30, 2021
@tofuya
Copy link
Author

tofuya commented May 31, 2021

As I recall, this is the only pattern of Class, Module, and Method that can be defined in single line in Ruby.

a) class Foo < Bar; end
b) module Baz; end
c) def foo; "hello" end
d) def bar() "hello" end
e) def baz(v) = "hello"

For a), b), c), d), It seems to be enough to determine that anything ending with "end" is a single-line-definition.
For the pattern in e), we only need to consider the Method, so if " =" (space and =) is included after def, we can assume it is a single-line-method.
As an aside, the reason for the space is that the following methods are very commonly used as Setters, and a line without a space should not be considered a single-line-method.

def name=
  ...
end

In summary, the following criteria seem to be good.

classification criteria
a) class ... end
b) module ... end
c), d) def ... end
e) def ... " =" ...

@jcs090218
Copy link
Member

jcs090218 commented May 31, 2021

Thanks for the suggestions! I have manage to make this to work! Can you upgrade to the latest version and see if it works? Thanks! 👍

@tofuya
Copy link
Author

tofuya commented May 31, 2021

There were a few things that bothered me.

Errors have occurred

class Foo < Bar
  def baz() = "hello"
end

Run origami-close-all-nodes
=> (error "Tried to construct a node where the children overl...")

class Foo
  def bar(v)
    print v unless v
  end
end

Run origami-close-all-nodes
=> (wrong-type-argument number-or-marker-p nil)

The class name will be hidden

スクリーンショット 2021-05-31 15 32 11

I dodn't think it was necessary to hide the class name. After all, I'll have to open everything because I won't be able to see the outlines.

@jcs090218
Copy link
Member

(error "Tried to construct a node where the children overl...")

Does the code valid in Ruby? Sorry I am not very familiar to Ruby code. The problem is class is map to the end but missing another end for def.

(wrong-type-argument number-or-marker-p nil)

Can you set debug-on-error and paste the backtrace here? Thanks!

Hiding the class name isn't intentional. As I said, the parser doesn't know where to start and end the folding. I manage to work to shift the starting of the folding region right after the match.

Before

(fold end) (fold start) class ClassName ... end  # folding error, overlaps

After

class (fold start) ClassName ... (fold end) end  # Current workaround

I propose shift another word towards to the right? So it should end up like

class ClassName(fold start) ... (fold end) end  # Current workaround

@tofuya
Copy link
Author

tofuya commented May 31, 2021

  1. It's valid since Ruby 3.0
    https://bugs.ruby-lang.org/issues/16746
def bar(v)
  print v if v
end

Run origami-close-all-nodes
=>

Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p nil)
  #f(compiled-function (node) #<bytecode 0x1ff415b676f1>)([1 2305843009213693951 0 t ([1 27 9 t ([11 21 10 t nil #<overlay from 1 to 1 in test4.rb>]) #<overlay from 1 t$
  origami-fold-find-deepest([1 2305843009213693951 0 t ([1 27 9 t ([11 21 10 t nil #<overlay from 1 to 1 in test4.rb>]) #<overlay from 1 to 1 in test4.rb>]) root] #f(co$
  origami-fold-find-path-containing-range([1 2305843009213693951 0 t ([1 27 9 t ([11 21 10 t nil #<overlay from 1 to 1 in test4.rb>]) #<overlay from 1 to 1 in test4.rb>$
  origami-fold-find-path-with-range([1 2305843009213693951 0 t ([1 27 9 t ([11 21 10 t nil #<overlay from 1 to 1 in test4.rb>]) #<overlay from 1 to 1 in test4.rb>]) roo$
  #f(compiled-function (beg end offset children) #<bytecode 0x1ff4147b5ab5>)(1 nil 10 ([12 26 14 t nil #<overlay from 26 to 26 in test4.rb>]))
  #f(compiled-function (positions) #<bytecode 0x1ff415b5af81>)(((#("def" 0 3 (face font-lock-keyword-face fontified t)) . 1) (#("if" 0 2 (face font-lock-keyword-face fo$
  origami-build-pair-tree(#f(compiled-function (beg end offset children) #<bytecode 0x1ff4147b5ab5>) "\\(s*def\\|class\\|module\\|if\\|unless\\|while\\|until\\|..." "\\$
  #f(compiled-function (content) #<bytecode 0x1ff4147b5aed>)(#("def bar(v)\n  print v if v\nend\n" 0 3 (face font-lock-keyword-face fontified t) 3 4 (fontified t) 4 7 ($
  #f(compiled-function (content) #<bytecode 0x1ff414624f2d>)(#("def bar(v)\n  print v if v\nend\n" 0 3 (face font-lock-keyword-face fontified t) 3 4 (fontified t) 4 7 ($
  origami-build-tree(#<buffer test4.rb> #f(compiled-function (content) #<bytecode 0x1ff414624f2d>))
  origami-get-fold-tree(#<buffer test4.rb>)
  origami-close-all-nodes(#<buffer test4.rb>)
  funcall-interactively(origami-close-all-nodes #<buffer test4.rb>)
  call-interactively(origami-close-all-nodes nil nil)
  command-execute(origami-close-all-nodes)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants