Skip to content
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

[WIP] Add support for CSS parsing and future categories. #62

Merged
merged 18 commits into from
Dec 7, 2015

Conversation

RReverser
Copy link
Collaborator

  • Added support for parser categories.
  • Added rework, postcss and cssom CSS parsers and moved JS and CSS parsers into corresponding categories.
  • Added correspoding UI switch and made drop target aware of category (auto-switches between JS/CSS parsers depending on file type).

Depends on #61 to be merged first, and might be a bit buggy yet, as I did only basic testing and help with it would be appreciated.

Also, TODO: add CSS transformers, but that's probably worth separate PR.


p {
background-color: yellow;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it's WIP so probably just wanted to get something working.

Might be good to leave the same kind of comments from the js example and maybe show off some more css ast nodes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, totally open to modifications, just needed something to play with.

@hzoo
Copy link
Collaborator

hzoo commented Dec 3, 2015

lol wow - screenshots 😄?

@RReverser
Copy link
Collaborator Author

lol wow - screenshots 😄?

As I said, need helo with testing, so it will be extra motivation to run locally for now :P

(if seriously, not near the laptop anymore today)

@fkling
Copy link
Owner

fkling commented Dec 3, 2015

Awesome, thank you so much! I'm pretty busy during the week but hopefully have some time over the weekend to take a closer look!

@RReverser
Copy link
Collaborator Author

One known issue that is left so far is not changing code when e.g. turning on transformer while CSS was activated. Unfortunately, app.js feels somewhat complicated due to various events that change state in different ways and I struggle to properly change all the relevant places :(

Maybe we should use smth like Redux for simplifying state management? (although I understand that switch might be not too easy, it's just that makes me sad how hard it's to add modifications to UI component of AST explorer compared to how easy is to add parsers)

@fkling
Copy link
Owner

fkling commented Dec 4, 2015

Yeah, it used to be a lot simpler :P

Switching to redux was also on my mind, but I simply haven't had the time yet to do anything in that direction.

@fkling
Copy link
Owner

fkling commented Dec 4, 2015

Do we need a category button? Couldn't we just keep a single parser button and list the different parsers in separate columns? The parser itself should already know to which category it belongs to.

Or would that make things more difficult in the long run?

@RReverser
Copy link
Collaborator Author

Initially I had nested menu, but yeah, it feels a bit harder to manage, especially for auto-choosing parser category from dropped file, adding new categories etc. But if you want to refactor it in the way those things will be still simple, I'm all for it.

@RReverser
Copy link
Collaborator Author

@fkling So, as for change of parser category on transformer - I'm leaving it for now, if you could help, it would be highly appreciated.

Also, I have implemented one more new category 😉 , but I'll better leave it until this is cleaned up and merged as this PR already got way too much changes :)

@fkling
Copy link
Owner

fkling commented Dec 4, 2015

Uh nice :) I can't say how much I appreciate your contributions thank you! I'll work on this tomorrow.

@RReverser
Copy link
Collaborator Author

I can't say how much I appreciate your contributions thank you!

Well it's fun to work on this kind of stuff for me too :)

I'll work on this tomorrow.

I'll likely be busy (or rather having some rest) tomorrow, but feel free to write any questions, I'll respond as soon as get free.

@RReverser RReverser changed the title [WIP] Add support for CSS parsing (rework, postcss) and future categories. [WIP] Add support for CSS parsing (rework, postcss, cssom) and future categories. Dec 6, 2015
@RReverser RReverser changed the title [WIP] Add support for CSS parsing (rework, postcss, cssom) and future categories. [WIP] Add support for CSS parsing and future categories. Dec 6, 2015
@RReverser
Copy link
Collaborator Author

@fkling Added some structure and UI amendments and CSSOM parser. Please stop me until I push everything to one PR :)

* Add getNodeName support to array-like objects which are not arrays.
* Hide `length` from any array-like objects.
* Show extra properties on array-like objects (similar to Node.js console.log).

This improves uglify-js, CSSOM and other outputs.
@RReverser
Copy link
Collaborator Author

One known issue that is left so far is not changing code when e.g. turning on transformer while CSS was activated.

Fixed this one in a different way - transformer list is now also limited only to chosen category in the same way as parsers, which both solves this issue and helps for adding future CSS transformers and so.

@fkling
Copy link
Owner

fkling commented Dec 6, 2015

transformer list is now also limited only to chosen category in the same way as parsers

Makes sense to me. I guess having a dedicated button for the category isn't such a bad idea after all.

Please stop me until I push everything to one PR

I'm trying to merge everything today, so please STOP :P ;)

@RReverser
Copy link
Collaborator Author

I'm trying to merge everything today, so please STOP :P ;)

Soooorry, one more fix :)

@RReverser
Copy link
Collaborator Author

Aaand... Done for now. There is one more specific scenario, which is not properly handled, but should be fine for now.

@RReverser
Copy link
Collaborator Author

Anyway, better to start with merging of #60 and #61 I guess as they are simple and important for further updates.

@RReverser
Copy link
Collaborator Author

Just to explain stuff related to require.context - this is a WebPack function that allows to perform dynamic requires by compile-time regular expression. I used it to centralize and simplify maintaining and adding parsers and transformers, so that at this point we have following folder structure:

src
  parsers
    {category}
      codeExample.txt
      {parser1}.js
      {parser2}.js
      ...
      {parserN}.js
      transformers
        {transformer1}
          codeExample.txt
          index.js
        ...
        {transformerN}
          codeExample.txt
          index.js

So that when adding new category, parser or transformer you don't need to modify a lot of imports and exports, you don't need to provide bunch of boilerplate in each parser and category or anything else - just add file to corresponding path and that's it (ok, if adding not a parser/transformer but entirely new category, you do need to import CodeMirror mode and define 2-line index.js metadata but that's just 3 lines 😄 ).

IMO, this should simplify adding new modes a lot and encourage people to contribute.

@fkling
Copy link
Owner

fkling commented Dec 7, 2015

That's pretty sweet :)

I started merging the changes locally. I made two additions:

  • Instead of storing the last used parser and using the default parser when switching categories, it now stores the last used parser per category.
  • With the introduction of different source code types (CSS, JS, etc), it makes sense the store the selected parser with the code snippet when (i.e. when pressing "save"). I made the corresponding changes in Parse.

I think I just have to merge the HTML stuff tomorrow and this should be good to go! :)

@RReverser
Copy link
Collaborator Author

Sounds great to me!

@RReverser
Copy link
Collaborator Author

Instead of storing the last used parser and using the default parser when switching categories, it now stores the last used parser per category.

Does it mean it stores last category, too? Or how does it restore it when refreshing the page?

@fkling
Copy link
Owner

fkling commented Dec 7, 2015

Does it mean it stores last category, too?

Yes, that too!

fkling added a commit that referenced this pull request Dec 7, 2015
[WIP] Add support for CSS parsing and future categories.
@fkling fkling merged commit a046d16 into fkling:master Dec 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants