Skip to content
This repository was archived by the owner on Sep 6, 2018. It is now read-only.

Add pug (jade) support#67

Closed
webdif wants to merge 1 commit into
atom:masterfrom
webdif:master
Closed

Add pug (jade) support#67
webdif wants to merge 1 commit into
atom:masterfrom
webdif:master

Conversation

@webdif

@webdif webdif commented May 8, 2016

Copy link
Copy Markdown

Before / after:

before-after

@lee-dohm

lee-dohm commented May 8, 2016

Copy link
Copy Markdown
Contributor

Would you mind creating the same change for https://github.com/atom/one-light-syntax?

@webdif

webdif commented May 9, 2016

Copy link
Copy Markdown
Author

Of course. The PR is here: atom/one-light-syntax#17

@lee-dohm

Copy link
Copy Markdown
Contributor

@simurai what do you think?

@simurai

simurai commented May 13, 2016

Copy link
Copy Markdown
Contributor

The styles seem fine. 👍

But not sure where we wanna draw the line what language should be supported in the default theme. Should it only be languages that come bundled by default with Atom? I guess every selector that gets added comes with a tiny tiny performance cost, even though you might never need those styles. Or is that so small that we shouldn't worry about it?

/cc @atom/languages @atom/feedback

@thomasjo

Copy link
Copy Markdown
Contributor

I'm personally leaning towards only adding support for languages bundled with Atom. At the same time, I realize that might not provide an optimal end-user experience...

@webdif

webdif commented May 13, 2016

Copy link
Copy Markdown
Author

Is it possible to add language support to a theme, via a separated package? That's what I searched for, before ending up with this PR.

@lee-dohm

Copy link
Copy Markdown
Contributor

@webdif Yes, it is. For example, here is some code from my styles.less that keys off of the theme name:

.theme-one-light-ui, .theme-one-dark-ui {
  .status-bar { font-size: 15px; }
  .tab-bar { font-size: 15px; }
  .tree-view { font-size: 15px; }
}

Atom adds the names of the themes as classes on atom-workspace so that you can do specifically this kind of customization.

On the flip side, a separate package isn't going to have access as easily to the color information you're using in this PR.

@simurai

simurai commented May 14, 2016

Copy link
Copy Markdown
Contributor

In this case, you could hard code the colors in your styles.less file:

.theme-one-dark-syntax atom-text-editor::shadow {
  .source.pug,
  .source.jade {
    .meta {
      &.tag.attribute {
        &.id {
          color: hsl(207, 82%, 66%);
        }
        &.class {
          color: hsl( 29, 54%, 61%);
        }
      }
      &.delimiter {
        color: hsl(  5, 48%, 51%);
      }
    }
    .constant.character.entity {
      color: hsl( 39, 67%, 69%);
    }
  }
}

Of course, the colors could change, unlikely anytime soon, but still. So another option is to try to use the official variables. Other themes might support them too and if not, there is a fallback from core:

@import "syntax-variables";

atom-text-editor::shadow {
  .source.pug,
  .source.jade {
    .meta {
      &.tag.attribute {
        &.id {
          color: @syntax-color-method; // @hue-2
        }
        &.class {
          color: @syntax-color-attribute; // @hue-6
        }
      }
      &.delimiter {
        color: @syntax-color-variable; // @hue-5-2
      }
    }
    .constant.character.entity {
      color: @syntax-color-class; // @hue-6-2
    }
  }
}

But it might not always be easy to map the right variables.

@simurai

simurai commented May 14, 2016

Copy link
Copy Markdown
Contributor

Options I can think of:

  1. Add support in styles.less. See above.
  2. Create a fork, maybe under https://github.com/atom-community/, like one-dark-plus-syntax and accept any language. Downside: Yet another theme to maintain. And it doesn't really solve the "I just need the styles for the languages I use and not everyone else's".
  3. Create a package like language-pug-themes. Then do same as above for styles.less. Downside: Then the question is, which themes should be supported. And with hardcoded colors it's probably too much work to maintain. With using the official variables it's hard to predict how themes style the rest.
  4. The language packages should add the styling. Especially in this case where there are two 1 2 equal popular language packages and after the name change from Jade -> Pug, maybe language-pug might become popular too. Then each language could fine tune the syntax highlighting on their own. Currently they just define the scopes and hope themes use it in the right way and like in this case, add support for it. But that would have to wait for 2.0 so that all themes support the same color variables that language packages can use.

@simurai

simurai commented May 18, 2016

Copy link
Copy Markdown
Contributor

For now I would suggest doing option 1 (styles.less) for non-bundled languages. And a wontfix for this PR.

Then when Atom 2.0 comes around, try to do option 4. So that when you install a language package, syntax highlighting is supported without having to hunt for the right theme or beg your favorite theme to add support.

@webdif Are you ok customizing it in your styles.less for now?

@webdif

webdif commented May 18, 2016

Copy link
Copy Markdown
Author

I'm OK with this 👍

@simurai simurai closed this May 18, 2016
@simurai simurai added wontfix and removed atom labels May 18, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants