Highlight Ruby method definitions without arguments #1523

cbothner · 2018-08-14T21:52:20Z

Inheriting from clike is enough to deal with constructs like this

def foo(bar)
  ...
end

but not

def foo # foo is not highlighted by prism
  …

end

Now, admittedly this does not highlight call sites. But since much idiomatic Ruby doesn’t use parentheses in function calls, call sites can’t be consistently highlighted anyway. This behavior is consistent with the ruby definitions used by at least vim, Atom, and Github (e.g. below)

def something
  foo(bar).baz
  Logger.info 'Hello, World!'
end

It is also the most internally consistent behavior: all method definitions have the name highlighted; no call sites do.

Inheriting from `clike` is enough to deal with constructs like this ```ruby def foo(bar) ... end ``` but not ```ruby def foo ... end ``` This does not highlight call sites, but since much idiomatic Ruby doesn't use parentheses in function calls, call sites can't be consistently highlighted anyway.

cbothner · 2018-08-14T22:26:01Z

Okay, I definitely forgot to consider the knock-on effects of my changes on languages that extend ruby. Before I go changing tests of other languages/defining “function” back to clike’s version in those language, I would like your opinion of whether this change is acceptable. I think the consistent lack of highlighting at call-sites is better than highlighting foo(bar) but not foo bar — this is what I’ve seen in editors and on this website. But others may disagree.

Additionally, I found that my PR is also still not ready to merge, since it broke static method definitions.

class Foo
  def self.bar # This is broken in my PR: <def> <self>.bar where <_> is “keyword”
  end
end

I can’t seem to make it look back past two other highlighted segments so I’d appreciate guidance.

RunDevelopment · 2018-08-15T09:31:24Z

Ok, I looked into it and the problem has to do with how greedy matching is currently implemented. We might need to change that behavior but for the time being, there is an easy workaround:

Your pattern requires that it can match the def keyword, so searching for functions before the keywords are matched solves the problem. There are some patterns which are inserted before keyword already, so you can insert your pattern (without greedy: true) after symbol.
Now, because of the way insertBefore is implemented, you will also have to delete Prism.languages.ruby.function before you insert your new function.

See [Crystal's docs][docs] where `@[Extern]` is highlighted the same as `@[Extern(union: true)]`. [docs]: https://crystal-lang.org/docs/syntax_and_semantics/attributes.html#extern

RunDevelopment · 2018-08-17T19:56:23Z

components/prism-ruby.js

@@ -13,7 +13,7 @@
 				greedy: true
 			}
 		],
-		'keyword': /\b(?:alias|and|BEGIN|begin|break|case|class|def|define_method|defined|do|each|else|elsif|END|end|ensure|false|for|if|in|module|new|next|nil|not|or|protected|private|public|raise|redo|require|rescue|retry|return|self|super|then|throw|true|undef|unless|until|when|while|yield)\b/
+		'keyword': /\b(?:alias|and|BEGIN|begin|break|case|class|def|define_method|defined|do|each|else|elsif|END|end|ensure|false|for|if|in|module|new|next|nil|not|or|protected|private|public|raise|redo|require|rescue|retry|return|self|super|then|throw|true|undef|unless|until|when|while|yield)\b/,


Probably just nitpicking, but could you please remove the additional comma?

RunDevelopment · 2018-08-17T20:14:50Z

components/prism-ruby.js

@@ -75,6 +77,10 @@
 		'symbol': {
 			pattern: /(^|[^:]):[a-zA-Z_]\w*(?:[?!]|\b)/,
 			lookbehind: true
+		},
+		'function': {
+			pattern: /(def )[\w.]+/,


I had a little fun with a Ruby REPL and found that this pattern will also happily match:

my_function_def variable

But at the same time, it doesn't match

def extra_spaces_and_new_line(arg) end

Changing the lookbehind group to (\bdef\s+) ought to solve these cases.

Good catch. (Although I’ve never seen someone put a newline between def and the method name)

Well, me either, but the compiler can handle it, so there is probably someone out there using it.

RunDevelopment · 2018-08-17T20:33:53Z

@cbothner

Two things concerning the insides of function names:

Punctuation: Right now . is considered punctuation, but it is not inside of function names.
Keywords: The self keyword is not highlighted.

Both are illustrated by your example:

def self.bar
end

cbothner · 2018-08-17T21:08:26Z

@RunDevelopment I had intended for class method definitions to be highlighted that way, since that’s how I’m used to seeing it in atom, vscode, Github, and Textmate. A more complete survey, however, reveals that’s far from universal: vim and pygment (which ruby-lang.org uses, and I’m taking that as an endorsement of its style) both highlight def self.foo with self and . different colors than foo. (Interestingly, both do def in a different color than self, but that’s not important)

Also to my horror I have found it is possible to do this

class Foo
  def String.bar
    42
  end
end

String.bar # => 42

RunDevelopment · 2018-08-17T21:45:19Z

@cbothner
Yeah, I see the problem. How about this:
We give up on function make it function-declaration (or a more fitting name) which will look like this:

'function-declaration': {
	pattern: /(\bdef\s+)[\w.]+/,
	lookbehind: true,
	inside: {
		'function': /\w+$/,
		rest: null
	}
}

and at the end of the language definition (after string) we include:

Prism.languages.ruby['function-declaration'].inside.rest = Prism.languages.ruby;

And like that, we should have a function-declaration wrapper for those who want one color, and the more detailed contents for anybody else (default).

(It seems to work but I haven't thoroughly tested it)

Sometimes definitions of class methods `def [self.foo]` are highlighted where everything in brackets is the same color; other times `self` and `.` are highlighted as keyword and punctuation. This adds a wrapper `method-definition` which allows users to choose.

cbothner · 2018-08-20T15:32:50Z

Ah, nice job thinking like a platform maker. Here I was thinking we had to make a decision, and your suggestion preserves the user’s a ability to choose. I like that.

I think I correctly understood your merged PR #1531 and didn’t use the workaround you initially proposed…

RunDevelopment · 2018-08-20T16:57:49Z

@cbothner
Yes, you did exactly the right thing.

mAAdhaTTah · 2018-12-01T20:34:47Z

@RunDevelopment How is this PR looking?

Ruby methods might or might be called in C-style creating inconsistent highlighting. This highlights only method definitions and removes the highlighting of C-style-invoked methods.

cbothner added 5 commits August 17, 2018 15:22

Add a test for method definition

2d2d9f1

Fix highlighting of Ruby static methods

d4695ca

Change Crystal attribute test

eee0b86

See [Crystal's docs][docs] where `@[Extern]` is highlighted the same as `@[Extern(union: true)]`. [docs]: https://crystal-lang.org/docs/syntax_and_semantics/attributes.html#extern

Change Crystal keyword test input

2946ba0

Change ERB and HAML tests to reflect Ruby changes in 8173ec6

1c7df91

RunDevelopment reviewed Aug 17, 2018

View reviewed changes

Remove trailing comma

5789c7d

RunDevelopment reviewed Aug 17, 2018

View reviewed changes

cbothner added 2 commits August 17, 2018 16:56

Fix multiline method definitions; don't match methods ending with "def"

5f60430

Highlight self as keyword, . as punct. in class method definitions

782ef55

RunDevelopment mentioned this pull request Aug 18, 2018

insertBefore now correctly updates references #1531

Merged

mAAdhaTTah added enhancement language-definitions needs review labels Aug 19, 2018

RunDevelopment approved these changes Dec 1, 2018

View reviewed changes

RunDevelopment merged commit 7277591 into PrismJS:master Dec 1, 2018

This was referenced Oct 26, 2019

[Snyk] Upgrade prismjs from 1.8.1 to 1.17.1 ajesse11x/css-houdini.rocks#2

Open

[Snyk] Upgrade prismjs from 1.16.0 to 1.17.1 ajesse11x/svelte#2

Open

snyk-bot mentioned this pull request Mar 8, 2020

[Snyk] Upgrade prismjs from 1.10.0 to 1.19.0 tjenkinson/lit-html#5

Open

snyk-bot mentioned this pull request Mar 21, 2020

[Snyk] Upgrade prismjs from 1.14.0 to 1.19.0 bangbang93/freyja#143

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Highlight Ruby method definitions without arguments #1523

Highlight Ruby method definitions without arguments #1523

cbothner commented Aug 14, 2018

cbothner commented Aug 14, 2018

RunDevelopment commented Aug 15, 2018

RunDevelopment Aug 17, 2018

RunDevelopment Aug 17, 2018 •

edited

Loading

cbothner Aug 17, 2018

RunDevelopment Aug 17, 2018

RunDevelopment commented Aug 17, 2018

cbothner commented Aug 17, 2018

RunDevelopment commented Aug 17, 2018

cbothner commented Aug 20, 2018

RunDevelopment commented Aug 20, 2018

mAAdhaTTah commented Dec 1, 2018

Highlight Ruby method definitions without arguments #1523

Highlight Ruby method definitions without arguments #1523

Conversation

cbothner commented Aug 14, 2018

cbothner commented Aug 14, 2018

RunDevelopment commented Aug 15, 2018

RunDevelopment Aug 17, 2018

Choose a reason for hiding this comment

RunDevelopment Aug 17, 2018 • edited Loading

Choose a reason for hiding this comment

cbothner Aug 17, 2018

Choose a reason for hiding this comment

RunDevelopment Aug 17, 2018

Choose a reason for hiding this comment

RunDevelopment commented Aug 17, 2018

cbothner commented Aug 17, 2018

RunDevelopment commented Aug 17, 2018

cbothner commented Aug 20, 2018

RunDevelopment commented Aug 20, 2018

mAAdhaTTah commented Dec 1, 2018

RunDevelopment Aug 17, 2018 •

edited

Loading