styled-components / vim-styled-components

Vim bundle for http://styled-components.com based javascript files.
300 stars 24 forks source link

[WIP] Rewrite #30

Closed fleischie closed 6 years ago

fleischie commented 6 years ago

As somewhat announced in #21:

Rewrite the syntax highlighting behavior for styled-components (and like-minded awesome CSS-in-JS libraries).

There are some minor details still necessary to address:

mxstbr commented 6 years ago

How do I test this locally with Plug?

epilande commented 6 years ago

@mxstbr Plug 'styled-components/vim-styled-components', { 'branch': 'rewrite' } Or Plug '~/projects/vim-styled-components'

akinsho commented 6 years ago

Just tried this branch and its amazing :sparkles:. Thanks for all the hard work!!

mxstbr commented 6 years ago

I might have to do some research on how urgent/necessary support for styled-jsx is.

I don't think styled-jsx makes much sense in this repo since their syntax is completely different from the way styled-components looks. If there already is a plugin for it, why not let people use that instead? :blush:

mxstbr commented 6 years ago

Thanks @epilande!

This rewrite works amazingly, sooo niiiiceeee! 😍 Great job @fleischie

screen shot 2017-09-25 at 11 02 19 pm
joeybaker commented 6 years ago

So much better!

One thing I noticed: it looks like the the last curly brace at the end of a react component is getting highlighted incorrectly:

nwaywood commented 6 years ago

I posted a minor issue about this over here https://github.com/styled-components/vim-styled-components/issues/23#issuecomment-332046345

Jmeyering commented 6 years ago

Looks great! a little hiccup with some really nested stuff here. but much improved 👍 image

fleischie commented 6 years ago

Hey y'all! :wave:

I appreciate all your feedback very much, thank you. 🙇


@joeybaker Do you have more context on the React component? Preferably a minimum working example.


@Jmeyering This is a tricky situation, as I cannot safely assume, that all possible jsTemplateStrings inside of a styledDefinition should also contain CSS-highlighting (sorry for the lingo, these are the names of the appropriate regions inside of the vim syntax files).

I may be able to hack something fierce together (for the people of course) like this:

// @vim-styled-components
`
  <arbitrary jsTemplateString but highlighted because of the preceeding comment>
`

This might do the trick, but I don't know, whether I feel comfortable with editor-specific comments in language-specific files. Discuss! ( 😃 )

mxstbr commented 6 years ago

@fleischie this is an error on @Jmeyering's part, they should be using the css helper there anyway:

const StyledNavItem = styled.div`
  /* ... */
  ${({ hasChildre }) => hasChildren && css`
    /* ... */
  `}
`

That is a) the way we recommend using styled-components (we might actually enforce this in the future) and b) it fixes syntax highlighting. No need for editor-specific comments.

Jmeyering commented 6 years ago

@mxstbr The css helper feels unnecessary here. It feels strange to include it in my bundle when it's not providing anything to me. (I'd definitely be interested in the discussion around requiring it.)

And in this particular case it does not fix highlighting either.

Jmeyering commented 6 years ago

image

mxstbr commented 6 years ago

The css helper feels unnecessary here. It feels strange to include it in my bundle when it's not providing anything to me. (I'd definitely be interested in the discussion around requiring it.)

You're including the code either way. The css helper allows you to use functions without getting the surprising side effect of seeing [Function foo] in your CSS :laughing:

We generally recommend using it everywhere.

Jmeyering commented 6 years ago

@fleischie I added #31 with an example of the nested issue, even with the css helper being used.

@mxstbr I'd be interested in discussing the css helper more over on the SC github at some point.

mxstbr commented 6 years ago

Sure, just open an issue!

joeybaker commented 6 years ago

@fleischie here you go! https://gist.github.com/joeybaker/a3efd0ed1ab0cdd746fd247eec98d9e6

epilande commented 6 years ago

I may be doing some weird things here....Syntax highlighting is breaking in the interpolation

image

On simple usage of styled-components, it works perfectly 💯

image

Nonetheless, great work @fleischie! Definitely looking much better! 👏

Jmeyering commented 6 years ago

@epilande I noticed the same issue, using the styled((props) => Component)` ` syntax does not highlight correctly

fleischie commented 6 years ago

@joeybaker Sorry, this snippet does not yield an error in my config. If you have an incorrectly highlighted closing bracket, can you run :echo SynEOL(<line-number>) and tell me, what it says?


@epilande and @Jmeyering the latest commits (especially 78b2ec3) should fix your mentioned erroneous highlighting. (I should mention, that for syntax highlighting I recommend using css`` instead of plain jsTemplateStrings.)

Jmeyering commented 6 years ago

This I believe falls under the category of "things you would be unable to check for" But if you are using "templates" like explained here

https://github.com/styled-components/styled-components/blob/master/docs/tips-and-tricks.md#more-powerful-example

You don't get highlighting, which is a regression from the old version.

image

fleischie commented 6 years ago

@Jmeyering That's the thing. The previous version checked for an import of a CSS-in-templateStrings library and highlighted evey templateString, which wasn't perfect, either.

It just took me 20min to figure out, what you mean by your example (I just might be too tired (for the people, though :wink:)).

Yeah, erm... what do you suggest? It might be possible to have some convoluted algorithm to store the defined css`` terms and highlight, if appropriate; or to check each string in a jsTemplateString and re-evaluate it, when certain CSS syntax is used. I don't even know, whether this is possible, let alone fix this specific issue.

If you have a good idea, let me know. 🤷‍♂️

akinsho commented 6 years ago

Been really enjoying the re-write branch although just observed some weird highlighting in a react component without styled-components in the project, seems to be following the use of the hypens and template literals. Once I commented out the plugin the highlighting returned to normal.

If it helps I use vim-jsx and pangloss/vim-javascript for syntax highlighting.

screen shot 2017-09-28 at 23 00 40
Jmeyering commented 6 years ago

@fleischie I really don't know ha!

It's tough because there are literally infinite ways to create a styled-component, attempting to detect them becomes an impossible task.

fleischie commented 6 years ago

@Akin, I might just rename the plugin to 'vim-weird-highlighting'. 😅 I will try to debug this on the weekend, I already have an assumption.

fleischie commented 6 years ago

Heyheyhey, I invested some time in better indentation-support.

Let me know, if there are still problems with this.

Next up: Suggestions for omnifunc. :smile:

Edit: I just found two cases, where the indentation doesn't work. So don't get your hopes up. It probably will take another 2 months or so for me to be able and have time enough to investigate and/or fix. *Insert roll-eyes-gif-emoji-thing here.*

mxstbr commented 6 years ago

This branch partially breaks JSX syntax highlighting for me. This is what it looks like with vim-styled-components enabled:

screen shot 2017-12-10 at 14 30 00

When I comment out the Plug 'styled-components/vim-styled-components', { 'branch': 'rewrite' } line it looks correct:

screen shot 2017-12-10 at 14 30 35

Using vim-jsx with vim-javascript. This is the most minimal code that reproduces the issue:

class CoffeeList extends React.Component<Props, State> {
  render() {
    return (
      <View style={{ flex: 1 }}>
        {coffees === null ? (
          <Text>Loading...</Text>
        ) : (
          <FlatList
            data={coffees}
            keyExtractor={coffee => coffee.id}
            overScrollMode="always"
            refreshControl={
              <RefreshControl refreshing={fetching} onRefresh={refetch} />
            }
            renderItem={({ item: coffee }) => (
              <CoffeeListItem onPress={this.selectCoffee} coffee={coffee} />
            )}
            style={{ flex: 1 }}
          />
        )}
        <Modal
          visible={!!selectedCoffee}
          animationType="slide"
          onRequestClose={this.closeDetailModal}
        >
          <CloseButton onPress={this.closeDetailModal} />
          {selectedCoffee && (
            <CuppingResults {...selectedCoffee} />
          )}
        </Modal>
      </View>
    );
  }
}
mxstbr commented 6 years ago

Also doesn't seem to support ReactNative rules?

screen shot 2017-12-10 at 14 36 13
fleischie commented 6 years ago

I will try to find some time to look into it. Thanks for the edge-case data. 😅

alexxmde-zz commented 6 years ago

I'm a complete newbie on the subject, it's just a question: What is the matter with implementing this like "Does this string looks like css? Highlight"

fleischie commented 6 years ago

Conceptually this is what is being done.

Technically there are some restrictions, though:

I am still warmly welcoming any PRs. So if you'd like just go over to the source files and see if you can help out. 😀

fleischie commented 6 years ago

@mxstbr @joeybaker @Jmeyering @Akin909 @nwaywood

After years of neglect I managed to pour some hours into the above-mentioned issues. I am fairly certain, that it doesn't make any sense to keep this in it's own branch. If you (or somebody else interested in this) does not raise their concerns I will merge this branch beginning of next week.

Thank you all for you continuing support and let's hope I will manage to fix more issues in less time. 😄 🎉

mxstbr commented 6 years ago

Both React and ReactNative look great 😍

screen shot 2018-02-01 at 15 00 45

screen shot 2018-02-01 at 15 00 18

Unfortunately, this still breaks JSX highlighting :cry:

screen shot 2018-02-01 at 15 01 35

fleischie commented 6 years ago

@mxstbr I cannot reproduce. Would you be so kind and send me your vim-config file? (Ideally only the necessary parts.)

fleischie commented 6 years ago

The latest commit implements css attribute highlighting, feedback more than welcome. ✌️