add node

[branchvincent]

Description

Adds a node item. Note this does overlap with nvm, but not everyone uses the latter.

Screenshots (if appropriate)

How Has This Been Tested

• I have tested using Linux.
• I have tested using MacOS.

Checklist

• I have updated the documentation accordingly.
• I have updated the tests accordingly.

[IlanCosman]

Seems good 👍 My biggest concern is that it might not be useful to most people. I know that the few times I've worked on a node project, I haven't cared what version I was working with, but perhaps that's abnormal.

I'm also concerned about the overlap with the nvm item. Granted, not everyone uses nvm, but for those who do this will end up showing the node version twice, which is silly.

For these reasons, I'd suggest having this be off by default, like powerlevel10k does. I'm open to being convinced otherwise though.

[kidonng]

I would suggest checking for more files/folders beside package.json, you can refer to Starship's default config.

In my own implementation I'm checking for test -f package.json || test (count *.{{,c,m}j,t}s) != 0, if it helps.

My biggest concern is that it might not be useful to most people.

I'd like to agree with you, but I enable this all the time so I can't decide 😄

I'm also concerned about the overlap with the nvm item. Granted, not everyone uses nvm, but for those who do this will end up showing the node version twice, which is silly.

Starship (again) doesn't have a nvm item but only a Node item. IMO that suits most people and is compatible with (any) nvm. Last but not least, another reason to enable the item all the time!

[branchvincent]

My biggest concern is that it might not be useful to most people.

Personally, I find it helpful since I work on a bunch of different microservices, each with a slightly different node version. But I'll be happy as long as I (and others) can enable it 😄

I would suggest checking for more files/folders beside package.json

👍 sounds good to me, I just copied powerlevel10k. Not to scope creep here, but it would be great if this was eventually configurable.

IMO that suits most people and is compatible with (any) nvm.

Completely agree, I think it makes more sense to have one language version item rather than an item for every single version manager out there. For example, I use fnm for node but all I really care about is the output of node --version (of course, assuming that my version manager is properly configured).

[IlanCosman]

Sorry for the delay, had to finish Finals 😅

Alright, yeah, I agree with you guys. So we remove nvm and add node.

I would suggest checking for more files/folders beside package.json... In my own implementation I'm checking for test -f package.json || test (count *.{{,c,m}j,t}s) != 0, if it helps.

Can I ask why? AFAIK basically every node project has a package.json.

And I don't want to do file globbing, mostly because its performance sucks. time begin; count *.js; end takes 300-400 microseconds on my new Mac. Yah that's not the end of the world, but it takes a very outsized amount of time for the value it adds, which as I said above I'm not sure is any at all.

Not to scope creep here, but it would be great if this was eventually configurable.

Configurable functionality isn't the Fish way, it's harder to maintain, and I don't really see much benefit here.

P.S. @kidonng test (count *.{{,c,m}j,t}s) != 0 can be simplified into count *.{{,c,m}j,t}s >/dev/null for a little speed boost.