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

feat(pack): adds angular pack #254

Merged
merged 1 commit into from
Jun 5, 2023
Merged

feat(pack): adds angular pack #254

merged 1 commit into from
Jun 5, 2023

Conversation

axelcalixte
Copy link
Contributor

Hello,

I noticed there wasn't any pack for Angular so I made one for myself.
Maybe it is useful to someone else.

@luxus
Copy link
Member

luxus commented Jun 3, 2023

does it make sense that it depends on the ts pack? @owittek @Uzaaft

@Uzaaft
Copy link
Member

Uzaaft commented Jun 3, 2023

I don't think so. I remember mehalter voting against it whenever @owittek did it in the deno pack.

@owittek
Copy link
Member

owittek commented Jun 3, 2023

I actually do the same with the all-in-one pack and since Angular is made for Node I think it's fine to depend on the default ts-pack.

Copy link
Member

@mehalter mehalter left a comment

Choose a reason for hiding this comment

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

LGTM! We just need to mention the angular treesitter plugin that gets installed. Is that a dependency for treesitter?

@owittek
Copy link
Member

owittek commented Jun 3, 2023

We might want to switch the branch for the plugin as described here: nvim-treesitter/nvim-treesitter-angular#4 (comment)

@axelcalixte
Copy link
Contributor Author

LGTM! We just need to mention the angular treesitter plugin that gets installed. Is that a dependency for treesitter?

@mehalter The angular treesitter parser isn't available via :TSInstall angular. That's why the plugin is needed.

We might want to switch the branch for the plugin as described here: nvim-treesitter/nvim-treesitter-angular#4 (comment)

@owittek I wasn't aware of this branch. Thank you !

Copy link
Member

@owittek owittek left a comment

Choose a reason for hiding this comment

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

LGTM! @Uzaaft @luxus We'll have to refactor when we add a js pack as discussed on discord

@owittek owittek requested a review from mehalter June 3, 2023 15:26
@mehalter
Copy link
Member

mehalter commented Jun 3, 2023

@owittek do you mean js or html+css web pack? I was saying on discord a minimal html+css pack without any JavaScript

@mehalter
Copy link
Member

mehalter commented Jun 3, 2023

I can do the refactor on Monday

@owittek
Copy link
Member

owittek commented Jun 3, 2023

@owittek do you mean js or html+css web pack? I was saying on discord a minimal html+css pack without any JavaScript

I already replied on discord but I meant the js pack with a working DAP which should be added at the same time as the html+css web pack. I just wanted to mention it since it would have to be refactored too.

Copy link
Member

@owittek owittek left a comment

Choose a reason for hiding this comment

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

Refactor to use the html-css base pack


- Adds [tsserver pack](https://github.com/AstroNvim/astrocommunity/tree/main/lua/astrocommunity/pack/typescript)
- Adds `[nvim-treesitter-angular](https://github.com/elgiano/nvim-treesitter-angular)` for `angular` Treesitter parser
- Adds `html` and `css` Treesitter parsers
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Adds `html` and `css` Treesitter parsers
- Adds [HTML & CSS support](../html-css)

Comment on lines 4 to 6
{
import = "astrocommunity.pack.typescript",
},
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{
import = "astrocommunity.pack.typescript",
},
{ import = "astrocommunity.pack.html-css" },
{ import = "astrocommunity.pack.typescript" },

Comment on lines 8 to 14
{
"nvim-treesitter/nvim-treesitter",
opts = function(_, opts)
if opts.ensure_installed ~= "all" then
opts.ensure_installed = utils.list_insert_unique(opts.ensure_installed, { "html", "css" })
end
end,
},
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{
"nvim-treesitter/nvim-treesitter",
opts = function(_, opts)
if opts.ensure_installed ~= "all" then
opts.ensure_installed = utils.list_insert_unique(opts.ensure_installed, { "html", "css" })
end
end,
},

Comment on lines 18 to 20
opts = function(_, opts)
opts.ensure_installed = utils.list_insert_unique(opts.ensure_installed, { "angularls", "html", "cssls" })
end,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
opts = function(_, opts)
opts.ensure_installed = utils.list_insert_unique(opts.ensure_installed, { "angularls", "html", "cssls" })
end,
opts = function(_, opts) opts.ensure_installed = utils.list_insert_unique(opts.ensure_installed, "angularls") end,

@mehalter mehalter force-pushed the main branch 2 times, most recently from 0aeb7d2 to b59a7aa Compare June 5, 2023 11:10
Copy link
Member

@mehalter mehalter left a comment

Choose a reason for hiding this comment

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

Did a refactor to move to using the html-css pack and also moved the elgiano/nvim-treesitter-angular to be a dependency of nvim-treesitter. Have tested locally and looks good to go! Thanks @axelcalixte !

@mehalter mehalter merged commit 5745eb0 into AstroNvim:main Jun 5, 2023
Uzaaft pushed a commit that referenced this pull request Jun 11, 2023
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.

5 participants