Ruby lexer does not highlight first symbol of keyword-style args #522

Closed
IronSavior opened this Issue Aug 13, 2015 · 7 comments

Comments

Projects
None yet
4 participants
@IronSavior

(I'm sorry I couldn't submit a patch--I had to abandon my efforts to figure out how to set up and build a Komodo extension and tweak the ruby UDL with very little success.)

The ruby lexer does not highlight the first hash key as a symbol when using "keyword-style" implicit hash syntax.

How to reproduce

This issue only affects the following two cases related to keyword-style method syntax. Both cases are common, but case 1 is so common that it is represented in nearly all recent ruby code.

  1. The first argument in a method call using keyword-style syntax. (ruby 1.9+)
  2. The first argument in a method definition using keyword-style syntax--both with and without default values, see notes below. (ruby 2.0+)

This is more concisely demonstrated in code:

# Case 1: method calls
obj.a_method arg1: 'val1', arg2: 'val2'  # arg2 is highlighted as a symbol, but not arg1

# Case 2a: method definition with defaults
def a_method( arg1: 'val1', arg2: 'val2' ) # arg2 is highlighted as a symbol, but not arg1

# Case 2b: method definition without defaults (indicates a required argument)
def a_method( arg1:, arg2:) # arg2 is correct, but only the colon after arg1 is highlighted
def a_method( arg1: , arg2:) # (space after arg1:) none of arg1: is highlighted

No problem for standalone hash-literals

I just want to clarify that there is no problem for brace-delimited hash-literals which use keyword-style syntax:

a = {arg1: 'val1', arg2: 'val2'} # Highlights correctly for both, very nice!

Affected versions:

Komodo Edit, version 9.2.0, build 15966, platform win32-x86. Built on Tue Aug 04 21:08:06 2015
Komodo Edit, version 8.5.4, build 14424, platform win32-x86. Built on Thu Aug 14 00:30:57 2014
(also any prior version that I've ever used--I don't believe this is a regression)

@Naatan

This comment has been minimized.

Show comment
Hide comment
@Naatan

Naatan Aug 14, 2015

Member

So I wasn't able to reproduce this at first because everything looked right on my screen, then I remembered @ervumlens Style Spy addon and tried that, issue immediately became apparent. Seriousl @ervumlens, awesome work on that!

screenshot at 10-57-53

@mitchell-as all yours :p

Member

Naatan commented Aug 14, 2015

So I wasn't able to reproduce this at first because everything looked right on my screen, then I remembered @ervumlens Style Spy addon and tried that, issue immediately became apparent. Seriousl @ervumlens, awesome work on that!

screenshot at 10-57-53

@mitchell-as all yours :p

@Naatan Naatan added the Type: Bug label Aug 14, 2015

@mitchell-as mitchell-as added this to the 9.2.2 milestone Aug 14, 2015

@mitchell-as mitchell-as self-assigned this Aug 14, 2015

@mitchell-as

This comment has been minimized.

Show comment
Hide comment
@mitchell-as

mitchell-as Aug 14, 2015

Member

Note to self: This is an issue with Scintilla's Ruby lexer, which has not been updated to accommodate this syntax. The end of the isHashRocketSuccessorColon method needs be updated to:

  1. Allow for a '('.
  2. Allow for a method name style.
  3. Allow for .identifier.
  4. Have a better method name to account for the new Ruby syntax.
Member

mitchell-as commented Aug 14, 2015

Note to self: This is an issue with Scintilla's Ruby lexer, which has not been updated to accommodate this syntax. The end of the isHashRocketSuccessorColon method needs be updated to:

  1. Allow for a '('.
  2. Allow for a method name style.
  3. Allow for .identifier.
  4. Have a better method name to account for the new Ruby syntax.
@IronSavior

This comment has been minimized.

Show comment
Hide comment
@IronSavior

IronSavior Aug 14, 2015

Hey thanks for the fast update, guys!

I also wanted to mention that there is a slightly different failure mode for case 2. When the default values are not in the method definition and there is no space after the colon, the trailing colon of arg1 is recognized as a symbol. I updated the issue to add an example.

Hey thanks for the fast update, guys!

I also wanted to mention that there is a slightly different failure mode for case 2. When the default values are not in the method definition and there is no space after the colon, the trailing colon of arg1 is recognized as a symbol. I updated the issue to add an example.

mitchell-as added a commit that referenced this issue Aug 14, 2015

scintilla: Ruby: Highlight keyword-style arguments correctly - fixes #…
…522

rn=

(integrated from master branch change 9.2.1-231-ge9bcc68 by Mitchell <mitchellb@activestate.com>)

@mitchell-as mitchell-as modified the milestones: 9.2.1, 9.2.2 Aug 14, 2015

@IronSavior

This comment has been minimized.

Show comment
Hide comment
@IronSavior

IronSavior Aug 18, 2015

I do appreciate the fast action on this, but I must point out that it's not quite there yet. Here is a screenshot from the latest nightly.

image

I do appreciate the fast action on this, but I must point out that it's not quite there yet. Here is a screenshot from the latest nightly.

image

@mitchell-as

This comment has been minimized.

Show comment
Hide comment
@mitchell-as

mitchell-as Aug 18, 2015

Member

I wasn't aware you could define methods with keyword-style arguments and no parentheses (it wasn't in your original set of examples).

The last method call was a corner case I hadn't thought of. Thanks for the report.

Member

mitchell-as commented Aug 18, 2015

I wasn't aware you could define methods with keyword-style arguments and no parentheses (it wasn't in your original set of examples).

The last method call was a corner case I hadn't thought of. Thanks for the report.

@mitchell-as mitchell-as reopened this Aug 18, 2015

mitchell-as added a commit that referenced this issue Aug 18, 2015

scintilla: Ruby: fix corner-cases in keyword-style highlighting - fixes
#522

(integrated from master branch change 9.2.1-234-ga4c4fb9 by Mitchell <mitchellb@activestate.com>)
@IronSavior

This comment has been minimized.

Show comment
Hide comment
@IronSavior

IronSavior Aug 18, 2015

I'm blown away! Very much appreciated! :godmode: 👍

I'm blown away! Very much appreciated! :godmode: 👍

@Defman21

This comment has been minimized.

Show comment
Hide comment
@Defman21

Defman21 Aug 18, 2015

Contributor

Thanks for the fix 👍

Contributor

Defman21 commented Aug 18, 2015

Thanks for the fix 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment