Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Ruby code is not getting the right colors. #252

Open

Description

@cristianstu

Description

Ruby code is not getting the right colors.
Light theme seems to be ok.

Steps to Reproduce

  1. Upload to atom >= 1.32.0
  2. Open any ruby file.

Expected behavior: Show the file like this

image

Actual behavior:

image

Reproduces how often: Always

Versions

Atom : 1.32.1
Electron: 2.0.9
Chrome : 61.0.3163.100
Node : 8.9.3

apm 2.1.2
npm 6.2.0
node 8.9.3 x64
atom 1.32.1
python 2.7.15+
git 2.19.1

Additional Information

Seems like syntax--ruby class is missing on hierarchy. Switching to Ruby On Rails syntax fix the problem.

Activity

  1. Arcanemagus commented on Nov 8, 2018

    @Arcanemagus

    Atom v1.32.0 enabled a new parsing system by default. If you have specific issues about how the syntax highlighting has changed that you feel are mistakes, please take a look at the open issues on atom/language-ruby. If the specific problem that you're experiencing is not already being tracked there, please open a new issue on that repository.

    If you think that this issue is related to this theme in particular could you outline what scopes aren't being colored?

    Tree-sitter is a new grammar system that enables a number of advanced features including more rigorous syntax highlighting, which means more tokens will be detected correctly or more correctly than in the past. This means that colors may change because of this new information. What we're mostly looking for are situations where the underlying scopes of tokens are incorrect, not that the color is different than it was before. Some differences may be expected, such as variables now being recognized the same in every location they appear instead of differently based on the context. In general, we don't consider that a bug.

    If you want to see how Tree-sitter identifies the token at the current cursor position, you can open the Command Palette using Cmd+Shift+P on macOS or Ctrl+Shift+P on other platforms, then search for and execute "Editor: Log Cursor Syntax Tree Scope". If the scope at the bottom is incorrect, please file an issue and describe what you expected instead.

    Thanks!

  2. cristianstu commented on Nov 8, 2018

    @cristianstu
    Author

    @Arcanemagus I think everything being identified correctly but is not getting the proper colors. (checked with Log Cursor)

    Class names, for instance. The color should be @yellow

    https://github.com/atom/solarized-dark-syntax/blob/c4ed1ac41a5dd2120b912a766f4989e5b346bc33/styles/syntax/ruby.less#L37

    is not being applied because it depends on parent container class syntax--ruby, but that class is missing in the hierarchy.

    I guess adding that class is a language-ruby responsibility, or it was before, but if they have changed that class, this package will need to change that styles, right?

    Thanks!

  3. simurai commented on Nov 14, 2018

    @simurai

    I can reproduce this as well. It seems that the example code of

    class Article < ApplicationRecord
      belongs_to :category, optional: true
    end

    doesn't get wrapped with the syntax--source syntax--ruby classes on the parent <span>:

    screen shot 2018-11-14 at 2 27 43 pm

    Is that intentional? It means that all the styling in https://github.com/atom/solarized-dark-syntax/blob/c8a93df669e7a9a8ca062eb35426911bf864ed3b/styles/syntax/ruby.less#L1 doesn't get applied.

    Versions

    Atom    : 1.34.0-nightly12
    Electron: 2.0.12
    Chrome  : 61.0.3163.100
    Node    : 8.9.3
    

    I'll move this over to language-ruby.

  4. Ben3eeE commented on Nov 14, 2018

    @Ben3eeE
    Contributor

    @simurai The missing syntax--source syntax--ruby should be fixed in Atom 1.32.2 and later via 0208636.

    Which version of the package is nightly12 using?

  5. Ben3eeE commented on Nov 14, 2018

    @Ben3eeE
    Contributor

    Running a build from latest master looks like this with solarized-dark-syntax:
    image

  6. simurai commented on Nov 14, 2018

    @simurai

    should be fixed in Atom 1.32.2 and later

    Nice! 🙌

    Which version of the package is nightly12 using?

    Ouch.. sorry, this is embarrassing. 😅 I had some other example code above

    class Article < ApplicationRecord
      belongs_to :category, optional: true
    end

    ... which then probably messed up the highlighting. OK, works also in nightly12. 👍 Closing! And thanks for the checking. @Ben3eeE

  7. josephchoe commented on Nov 19, 2018

    @josephchoe

    If some of the colors still don't look right on the solarized dark theme, but the syntax--ruby classes are being populated on the hierarchy, does that mean this issue needs to go back to the atom/solarized-dark-syntax repository?

    For example:

    screen shot 2018-11-19 at 2 18 35 am

  8. simurai commented on Nov 20, 2018

    @simurai

    @josephchoe does that mean this issue needs to go back to the atom/solarized-dark-syntax repository?

    Yes, it seems that require gets intentionally overridden here: https://github.com/atom/solarized-dark-syntax/blob/74bc0df24ca7493d9d8bcfcaedccb5cc05c2c2f9/styles/syntax/ruby.less#L123-L125

    Removing it, makes require blue. Not sure if that is prefered?

    image

  9. josephchoe commented on Nov 23, 2018

    @josephchoe

    I think it's supposed to be like this? Though I might be remembering wrong.

    screen shot 2018-11-22 at 9 57 07 pm

  10. simurai commented on Dec 3, 2018

    @simurai

    @josephchoe does that mean this issue needs to go back to the atom/solarized-dark-syntax repository?
    @josephchoe I think it's supposed to be like this?

    Sorry, you're right. The scopes seem to be different:

    Before (TextMate) After (Tree-sitter)
    textmate tree-sitter
    solarized-textmate solarized-tree-sitter

    But it's different for all themes, not just Solarized. Not sure if it's possible to change the Tree-sitter grammar so that it matches TextMate's?

    I'll re-open it here. It might be a duplicate of any of these tree-sitter .

  11. reopened this on Dec 3, 2018
  12. maxbrunsfeld commented on Dec 4, 2018

    @maxbrunsfeld
    Contributor

    I think it's supposed to be like this?

    IMO the new behavior makes more sense. require isn't a keyword; it's a built-in function, so I think support.function is the right scope. If solarized doesn't highlight support.function, then I think a lot of languages are going to look wrong, not just Ruby, because that's the scope used for "special" functions in many languages.

  13. gsmetal commented on Apr 12, 2019

    @gsmetal

    Hello, any updates here? There are a lot of problems with highlighting on tree sitter. For example:
    image

    1. Aren't include and extend built-in functions same as require?
    2. Function's argument with assignment is not highlighted as argument.
    3. super is not highlighted at all.
    4. Last var is highlighted as function, but it's definitely not a function.
    5. ...

    Edited: I am using atom 1.36.0, it's monokai syntax theme, but that problems on all themes including builtin.

  14. Ben3eeE commented on Apr 12, 2019

    @Ben3eeE
    Contributor
    1. Aren't include and extend built-in functions same as require?

    I don't know Ruby so I can't answer this question but if they are you can add them here and change it from exact to match so it's a regex. There are quite a few built in functions that are not scoped.

    {exact: 'require', scopes: 'support.function'}

    1. Function's argument with assignment is not highlighted as argument.

    This should be easy to fix by just adding:
    'optional_parameter > identifier': 'variable.parameter.function'

    Here:

    'method_parameters > identifier': 'variable.parameter.function'

    1. super is not highlighted at all.

    Same thing. Add a line for it 'super': 'the-scope.i-want' or '"super"': 'the-scope.i-want'. The one that works is the correct one.

    1. Last var is highlighted as function, but it's definitely not a function.

    Not as easy to fix as the other things.

    1. ...

    ...

    PRs are welcome to add these things.

  15. chbk commented on Apr 28, 2021

    @chbk
    Contributor

    I believe this can be closed when #291 is merged.

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions