Option to Show Node Version Only In Project #760

weilbith · 2018-02-19T21:37:53Z

As the vcs segment only appears in directories which are part of such a repository, it should be possible to set the segment node_version to appear only in NodeJS projects.

Functionality

By using a new variable called POWERLEVEL9K_NODE_VERSION_PROJECT_ONLY (which is set to false per default), the user can enable the described option. If so, the segment search for a project description file package.json from the current working directory upwards to the root. The search stops if a project root has been found. Afterwards the NodeJS version is just requested if this option is disabled or a project is detected.

Using this new variable and set it to false per default, the segment stay compatible to older configurations by the user.

Documenation

The documentation has been already adjusted in the README, where a new section for this segment has been opened, the option explained and linked in the list of all segments at the top of the document.

Testing

It is open to define tests. So far it has been practically tested on Ubuntu 16.4 with oh-my-zsh. It is not expected, that different OS and/or frameworks affect this behaviour.
This segments has no own test cases yet. To do so, an adjustment of the environment is required, to initialize a plain NodeJS project and check the behaviour in- and outside of this directory.

If this option is set, the segment is only shown inside node projects. Such a project is identified by a package.json description. The search is reverse from the current working diretory to the root. The search stops if such a description has been found. Add condition to show the segment, depending on the option variable and the search result.

Write a simple single line description. Create a table for the `PROJECT_ONLY` option. Link the section in the list of all segments.

weilbith · 2018-02-19T21:44:20Z

I've decided to implement such a search functionality, cause I'm not aware of any different option to determine a NodeJS project. Also a quick research do not offer such a possibility. As long as the current working directory is not that much nested, this search loop could be evaluated rly quick and should not represent an performance issue.

onaforeignshore · 2018-02-22T18:06:17Z

powerlevel9k.zsh-theme

@@ -1023,9 +1023,9 @@ prompt_load() {
  # Replace comma
  load_avg=${load_avg//,/.}

-  if [[ "$load_avg" -gt $(bc -l <<< "${cores} * 0.7") ]]; then


These are changes to prompt_load so muddies your PR. Would it be possible to do this in a separate PR if you want to suggest these changes, and also document what the changes are and why?

That have to be a slip. Wasn't my intention to touch this. I will revert this immediately.

weilbith · 2018-02-24T14:22:09Z

Still left the question, if the test environment should been adjusted for this.

bhilburn · 2018-03-07T03:00:43Z

Hey @weilbith - this looks great! I apologize for the long delay in responding to this PR. It's a great enhancement to the NodeJS segment, and I'm excited about getting it merged.

I think the point you raised in your initial post is a good one - this segment really needs its own tests, and we currently lack that. Are you willing to implement some test cases as part of this PR, @weilbith? It would be a great improvement for P9k :)

weilbith · 2018-03-07T07:03:28Z

@bhilburn sure, it's my implementation so should provide the tests for it too. :)

What r ur thoughts? What should I add? Simply a new test class in test/segments/?

bhilburn · 2018-03-07T22:00:09Z

@weilbith - Yup! I would make a new spec file and just fill in some cases that will give us confidence that if someone touches something that impacts the NodeJS segment down the road, we'll catch any breakages before it gets merged.

weilbith · 2018-03-26T04:58:18Z

@bhilburn Sry for beeing idle so long here.
I just read my first tutorial for shunit2 and it feels familiar. Just to make this sure: Running the shell unit tests is independent from the whole test-vm/test-bsd topic? A quick search in the whole repo where shunit is used make it look like they segment test classes are executed manually only. Did I miss something?

dritter · 2018-03-26T06:33:05Z

Running the shell unit tests is independent from the whole test-vm/test-bsd topic?

Yes. The test VMs are for manual tests if someone with BSD has a problem.

A quick search in the whole repo where shunit is used make it look like they segment test classes are executed manually only. Did I miss something?

No. The tests are also executed on every push by Travis-CI. Have a look at .travis.yml in the top folder. You need to add your spec there as well.

weilbith · 2018-03-26T07:00:39Z

@dritter
Thanks!
That make sense actually. ^^
Can I directly add the node package to travis environment? Cause it is required for my tests I guess.

weilbith · 2018-03-26T07:01:07Z

That was more "am I allowed to" and not how can I..m

bhilburn · 2018-03-26T18:21:43Z

@weilbith - Yes! You may add Node to travis for your testing =)

dritter · 2018-03-26T19:28:05Z

Hmm. I am opposed to installing node on travis. This would be an integration test. For a proper integration test we need to test different node versions (the node output could change between versions). This would lead to quite some complexity and make the tests more fragile (install X packages in Y different versions on Travis).
My suggestion is to mock node away, as our contract with node is the output (no need for a real node version). As a bonus: It would be easy to mock/test different node versions.
I did a similar thing in the go test.

bhilburn · 2018-03-26T19:37:11Z

@dritter is our test / travis / intergration expert, so I would like to go with his guidance, here. Let's do as @dritter suggests =)

weilbith · 2018-03-27T09:50:38Z

@dritter @bhilburn
Okay, accepted. But one point is left here: The project. Of cause I could do a whitebox test and simply do something like touch package.json, cause I now my own written extension is searching for this indicator. But this would not report us, if this doesn't work anymore. So if NodeJS should ever change their project structure definition, we don't get this. But as @dritter said, this lead to the trouble of working with different NodeJS versions and so forth.
So I fake both parts here, the version output and the project itself, where the segment should have an output. Correct?

dritter · 2018-03-27T19:14:44Z

@weilbith You are right. This is the downside of this approach. But if NodeJS chooses a different file name it would be a major BC break for them. I count on the community that would bring that change to us and change the P9K accordingly. And, as said, we would have to support two versions of NodeJS, which would then need a special setup (if we test against real installations).

So I fake both parts here, the version output and the project itself, where the segment should have an output. Correct?

Yes, you have to fake the node output and the node project (package.json). You can see a similar test here: testTruncateWithPackageNameWorks.

weilbith · 2019-03-21T11:55:18Z

Obsolete by more recent PR #1219 since I never made it extend the tests. 💀

weilbith added 2 commits February 19, 2018 22:07

Add section for the node_version segment.

3c30a9f

Write a simple single line description. Create a table for the `PROJECT_ONLY` option. Link the section in the list of all segments.

onaforeignshore reviewed Feb 22, 2018

View reviewed changes

Fix accidently changes in the load segment.

9d7c466

Merge branch 'next' into node-version_project-only_option

2ad5d2c

bhilburn assigned weilbith Mar 7, 2018

bhilburn added enhancement next labels Mar 7, 2018

ChrisBaker97 mentioned this pull request Mar 18, 2019

[Enhancement] adds ...PROJECT_ONLY option to node_version.p9k #1219

Open

weilbith closed this Mar 21, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Option to Show Node Version Only In Project #760

Option to Show Node Version Only In Project #760

weilbith commented Feb 19, 2018

weilbith commented Feb 19, 2018

onaforeignshore Feb 22, 2018

weilbith Feb 24, 2018

weilbith commented Feb 24, 2018

bhilburn commented Mar 7, 2018

weilbith commented Mar 7, 2018

bhilburn commented Mar 7, 2018

weilbith commented Mar 26, 2018 •

edited

Loading

dritter commented Mar 26, 2018

weilbith commented Mar 26, 2018

weilbith commented Mar 26, 2018

bhilburn commented Mar 26, 2018

dritter commented Mar 26, 2018

bhilburn commented Mar 26, 2018

weilbith commented Mar 27, 2018

dritter commented Mar 27, 2018

weilbith commented Mar 21, 2019

Option to Show Node Version Only In Project #760

Option to Show Node Version Only In Project #760

Conversation

weilbith commented Feb 19, 2018

Functionality

Documenation

Testing

weilbith commented Feb 19, 2018

onaforeignshore Feb 22, 2018

Choose a reason for hiding this comment

weilbith Feb 24, 2018

Choose a reason for hiding this comment

weilbith commented Feb 24, 2018

bhilburn commented Mar 7, 2018

weilbith commented Mar 7, 2018

bhilburn commented Mar 7, 2018

weilbith commented Mar 26, 2018 • edited Loading

dritter commented Mar 26, 2018

weilbith commented Mar 26, 2018

weilbith commented Mar 26, 2018

bhilburn commented Mar 26, 2018

dritter commented Mar 26, 2018

bhilburn commented Mar 26, 2018

weilbith commented Mar 27, 2018

dritter commented Mar 27, 2018

weilbith commented Mar 21, 2019

weilbith commented Mar 26, 2018 •

edited

Loading