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

Integrate melody-* packages into this project directly #26

Closed
zackad opened this issue May 4, 2024 · 7 comments
Closed

Integrate melody-* packages into this project directly #26

zackad opened this issue May 4, 2024 · 7 comments

Comments

@zackad
Copy link
Owner

zackad commented May 4, 2024

This project depends on several melody-* packages that hasn't been updated for awhile. I don't think that forking and re-publishing those packages is not an ideal solution. Let's just import it directly into this project.

@zackad
Copy link
Owner Author

zackad commented May 5, 2024

Dependencies_for_zackad_prettier_plugin_twig_melody_dependencies

From dependencies graph above, we need to pull this packages:

  • melody-code-frame
  • melody-parser
  • melody-extension-core
  • melody-types
  • melody-traverse

@rellafella
Copy link
Collaborator

I have to ask, is the purpose of this repo to have a continued development on a prettier plugin for melody? or was the original project an "almost there" solution for formatting twig templates and you want to use it as a starting point?

If it's the latter option, and you're looking to create a prettier plugin specifically for twig, and melody support isn't really critical, I have some thoughts.

I would have to say that the most salvageable parts of the original project would have to be the printer and the tests.

I do wonder if a project such as Twig lexer could potentially be a more suitable starting point for returning AST.

Not having worked within the melody framework I am just speculating, but it seems like each template is designed to have a .melody.twig file sitting alongside a .js file. The reason I believe that this is an important point is that the .melody.twig templates aren't supposed to have any kind of data manipulation going on, hence the absence of arrow functions in the parser.

Happy to move this to a discussion or discuss further on discord, but personally what I am looking for is a prettier plugin for twig files that makes no assumptions about the "flavor" of twig that your project is running, would you be happy for this project to go in that direction?

@zackad
Copy link
Owner Author

zackad commented May 6, 2024

To be honest, I don't even know how this project even works. All I want is a tool that would reformat twig in a consistent and sensible way. Unfortunately this is the only tools I found so far that fulfill my need.

I have to ask, is the purpose of this repo to have a continued development on a prettier plugin for melody? or was the original project an "almost there" solution for formatting twig templates and you want to use it as a starting point?

Personally I want to drop all melody stuff from this project and focus only on twig template. But at the same time I don't want to discredit the original project/author.

I would have to say that the most salvageable parts of the original project would have to be the printer and the tests.

I do wonder if a project such as Twig lexer could potentially be a more suitable starting point for returning AST.

parser, lexer, AST and such is a foreign concept to me. I have no idea how they work together. That's why there's not much activity on this project since I'm still learning.

By the way, are you interested to become maintainer of this project? Seems like you are more familiar on this topic than I do. Your contribution is greatly appreciated.

Happy to move this to a discussion or discuss further on discord, but personally what I am looking for is a prettier plugin for twig files that makes no assumptions about the "flavor" of twig that your project is running, would you be happy for this project to go in that direction?

I prefer discussing in the open forum like this, so everyone can participate or atleast see what has been discussed without creating yet another account.

@rellafella
Copy link
Collaborator

Don't get too carried away this is all pretty foreign to me too. I have had about 3 shots at building a robust formatter for twig so far, but each time it's been too much to do on my own so happy to work through it with you. If you'd like me to help maintain the project I would be happy to, otherwise also happy to submit some PRs where I can.

@zackad
Copy link
Owner Author

zackad commented May 7, 2024

Of couse I'm gonna stick around to make sure that this tool work properly since I'm use it daily. For now I've invite you as collaborator.

Don't feel burdened by this role. I just want to make sure that when I'm busy with other thing, there's someone who can takeover managing PR.

@rellafella
Copy link
Collaborator

rellafella commented May 8, 2024

Thanks mate. If we are going down the path of targeting this directly at twig rather than melody. How do you feel about renaming the package to avoid confusion.
I would propose keeping some attribution in the README regarding the origins of a lot of the code so that the original author is still attributed.

I would probably aim for a 1.0.0 release perhaps you would be able to setup a milestone and maybe a discussion and we could discuss in there what that could look like

@zackad
Copy link
Owner Author

zackad commented May 8, 2024

Agree with package renaming, which means we need to publish as separate package. AFAIK we can't rename published package on npm. I'm not sure if we should publish the new package name as-is right now or wait until release 1.0.

I've setup milestone and enable discussion. But I'm not sure if it would be any different that using github issue. Maybe I'm not aware of different use case, never use it.

zackad added a commit that referenced this issue Jul 18, 2024
Related: GH-26, GH-32

This project has external dependencies that no longer updated. Instead
of forking and probably re-releasing it as separate packages it might be
easier if we integrated it directly. This way, we don't have to manage
separate project.
zackad added a commit that referenced this issue Jul 18, 2024
Related: GH-26, GH-32

This project has external dependencies that no longer updated. Instead
of forking and probably re-releasing it as separate packages it might be
easier if we integrated it directly. This way, we don't have to manage
separate project.
zackad added a commit that referenced this issue Jul 18, 2024
Related: GH-26, GH-32

Partially import melody codebase directly into core.
@zackad zackad closed this as completed in b7a4a8e Jul 18, 2024
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

No branches or pull requests

2 participants