Skip to content

chore: added support for pnpm catalog entries#170

Open
stephansama wants to merge 11 commits intovuki656:masterfrom
stephansama:feature/add-catalog-lens
Open

chore: added support for pnpm catalog entries#170
stephansama wants to merge 11 commits intovuki656:masterfrom
stephansama:feature/add-catalog-lens

Conversation

@stephansama
Copy link
Copy Markdown

add pnpm catalog support

requires a dependency on yq to work properly however i think it is a small dependency for such a cool feature.

addresses this issue

image

Copy link
Copy Markdown
Owner

@vuki656 vuki656 left a comment

Choose a reason for hiding this comment

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

Although this all looks good I'm not a fan of this pnpm specific thing being so tightly coupled in in the main "generic" setup

Specifically stuff in virtual_text.lua. Not sure exactly how i'd like it to work or be laid out. If you're up for it try to refactor so virtual_text does't know about pnpm

If not I'll merge it and play around with it later on

Thanks for the PR

job({
json = true,
command = "npm outdated --json",
command = has_workspace() and "pnpm outdated --json" or "npm outdated --json",
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Lets extract this to a fn above/below somewhere now that we need to support 2 cases

Comment on lines +9 to +11
if string.find(value, "catalog:") ~= nil then
return true
end
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Move this to a fn above called is_pnpm_version or something similar since string find for catalog is a bit ambiguous

@stephansama
Copy link
Copy Markdown
Author

Although this all looks good I'm not a fan of this pnpm specific thing being so tightly coupled in in the main "generic" setup

Specifically stuff in virtual_text.lua. Not sure exactly how i'd like it to work or be laid out. If you're up for it try to refactor so virtual_text does't know about pnpm

If not I'll merge it and play around with it later on

Thanks for the PR

of course sir!

im not the greatest lua programmer hence why i converted my nvim config partially to typescript. but i will do my best to address the requested changes!

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.

2 participants