New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Combined keywords #22

Open
wants to merge 5 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@frenic
Copy link
Owner

frenic commented Apr 4, 2018

Combined literals would expand many properties and get rid of the | string. I did some smaller try outs but I realized that the syntax parser needs to group by combinator precedence for this to become less complicated. The real challenge after that is to combine different combinators with different multipliers. But I would be happy with just something to start with as long as nothing but | string is removed and no literals as of today.

  • Add tests
  • Group syntax entities by combinator precedence
  • Primary goal: Get rid of | string from display
  • Secondary goal: Get rid of | sting from align-content

Fixes #8

@frenic frenic force-pushed the combined-keywords branch 4 times, most recently from db9f7a8 to 80512fe Apr 5, 2018

@pelotom

This comment has been minimized.

Copy link
Collaborator

pelotom commented Apr 5, 2018

It looks like align-self also needs this treatment?

@frenic

This comment has been minimized.

Copy link
Owner Author

frenic commented Apr 5, 2018

The syntax is quite alike align-content so I guess it will solve itself when it's done 😃

@frenic frenic force-pushed the combined-keywords branch 2 times, most recently from 054dcce to c89f755 Apr 6, 2018

@frenic

This comment has been minimized.

Copy link
Owner Author

frenic commented Apr 6, 2018

Syntax entities are now grouped by combinator precedence. The identification whether a component should be included or not is simplified but will likely be revised when component combinations will be implemented.

I'm a bit surprised that this didn't cause any change to the definitions. You know that feeling when you've no clue why it works and you're convinced that you're not doing it completely right. But when you fix it, the output is completely identical. 😆

@frenic frenic force-pushed the combined-keywords branch from c89f755 to 313f2d1 Apr 6, 2018

@frenic frenic force-pushed the combined-keywords branch from 313f2d1 to 3efdd20 Apr 13, 2018

@frenic frenic force-pushed the combined-keywords branch from 3efdd20 to 34ca26c Apr 17, 2018

@frenic frenic force-pushed the master branch 6 times, most recently from 867f5cb to a7fbd63 Apr 26, 2018

@frenic frenic force-pushed the combined-keywords branch from 34ca26c to b885649 Jul 15, 2018

@pelotom

This comment has been minimized.

Copy link
Collaborator

pelotom commented Aug 1, 2018

In case this is proving hard to implement, allow me to propose something simple: I think most people who care about type safety, would be willing to forego the ability to use combined literals in exchange for getting rid of these | string fallbacks. What if there was simply an option to disable them? This could either take the form of a type parameter to all the interfaces, or more simply, a global Options interface which controls this (and maybe other things in the future):

interface Options {
}

this could then be overridden by the user with module augmentation:

declare module 'options' {
  export interface Options {
    allowCombinedKeyword: true;
  }
}

and finally used in a conditional type expression to determine if the | string fallbacks should be used or removed due to the presence of that option field.

@frenic

This comment has been minimized.

Copy link
Owner Author

frenic commented Aug 1, 2018

It's a bit complicated to implement, yes. But I've made some progress and I'm planning to resume that work after I my vacation. Hopefully! 😸

That case you're suggesting doesn't make things a lot easier. We would need to be selective and ignore | string for only certain properties. We cannot just remove | string everywhere. To automatically identify these properties would be nearly impossible because we would need some kind of statistics to determine how often combinations are used. Doing it manually ruins the whole idea of everything being automated and would require us to maintain that list depending on global usage over time.

I would rather put my efforts in finishing this. Sorry.

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