atom / language-ruby

Ruby package for Atom
Other
100 stars 144 forks source link

Add some missing keywords to Tree Sitter's keyword.control scope #272

Closed npezza93 closed 5 years ago

npezza93 commented 5 years ago

Requirements

Description of the Change

Highlight the below keywords as keyword.control. Example snippet:

alias name full_name
alias_method :name, :full_name
block_given?
defined? :thing
iterator?
redo
super
undef

Alternate Designs

None

Benefits

More accurate highlighting

Possible Drawbacks

None

Applicable Issues

N/A

rsese commented 5 years ago

Thanks @npezza93! Specs for the Tree-sitter grammar for language-ruby were recently added in https://github.com/atom/language-ruby/commit/c616809070ec6e5ff62d2144f43258b069d932c2#diff-336e0e422308721e5f4dc0cdc311417f - using those as a starting point, would you be up for adding tests for your changes?

npezza93 commented 5 years ago

@rsese Yep! Should be good to go now!

jasonrudolph commented 5 years ago

Thanks for this, @npezza93! :zap::bow:

We'll publish a new release of language-ruby including these updates :soon:.

I ended up removing the logic related to redo for now, because it didn't seem to be working correctly in my local testing. For example, I tested it with the following Ruby code:

i = 1
while i < 5
  puts i
  redo if [true, false].sample
  i += 1
end

The grammar doesn't seem to apply the keyword.control scope to redo:

Screen Shot 2019-08-27 at 3 58 08 PM

Instead of holding up the whole pull request, I pushed up 32a486f914026a3dafb91fd70950ef260c934828 to remove the redo bits for now.

If you're interested in trying to get redo to tokenize, I sketched out this test to demonstrate the current issue:

  it('tokenizes redo', async () => {
    const editor = await atom.workspace.open('foo.rb')

    editor.setText(dedent`
      i = 1
      while i < 5
        puts i
        redo if [true, false].sample
        i += 1
      end
    `)

    expect(editor.scopeDescriptorForBufferPosition([3, 2]).toString()).toBe(
      '.source.ruby .keyword.control'
    )
  })
npezza93 commented 5 years ago

Ok awesome, thanks for that @jasonrudolph! I'll take a look