Skip to content

Conversation

fj0r
Copy link
Contributor

@fj0r fj0r commented Apr 29, 2023

  • new decorator type |> for head of left
  • more convenient analysis

- new decorator type `|>` for head of left
- more convenient analysis
- get_component
- when there is only one component in left, '>>' takes precedence
- use `reduce` instead of `each & filter` (improved performance significantly)
@sholderbach
Copy link
Member

Can you please give the PR a more descriptive title so the people that might want to take a look can find it.

@fdncred fdncred changed the title refactor refactor powerline script Apr 29, 2023
@fdncred
Copy link
Contributor

fdncred commented Apr 29, 2023

@fj0r We can land this one, since I've been following your work, but rather than 100 PRs, maybe you should try to get things working the way you want and then submit a PR. Also, like I've said before, we need more descriptive text about what you're doing in the PR and with a proper title. Thank you for understanding!

@fdncred fdncred merged commit 4e07d66 into nushell:main Apr 29, 2023
@fj0r
Copy link
Contributor Author

fj0r commented Apr 29, 2023

Sorry. I'm not sure how detailed this description is, it's probably longer than the code if you don't miss relevant points.

  • measure
    • Put the most common patterns into power analyze
    • Use a separate file power_time.log to record data related to this module
    • When setting the environment variable NU_POWER_BENCHMARK to start recording, clear the previous records to avoid interference
    • Record the performance of each components, because sometimes that's the root cause
      • Recently, the optimization of kube stat and git_stat has had a good effect
      • Using return has weird results in some scenarios, I'm not sure, there is a commit for this issue
  • optimization
    • Replace the combination of each and filter with reduce (in left(right)_prompt and git_stat). I didn't expect there to be have such a big impact on performance
    • For kube stat, because kubectl itself takes tens of milliseconds to execute. So the execution result is cached. When the cache is valid, each execution only needs to determine that the information source file is older than the cache file, and then read the cache file, and the time does not exceed one millisecond

Because the prompt response is slow, which greatly affects the experience, so this part is more important

I'm not sure if this is verbose as these are obvious things

Overall, I thought it was just a little toy, as long as it finally works, there is no need to pay too much attention to the details of its internals. (That is to say, I thought that not many people would pay attention to this thing, until one day, they saw someone using powerline in zsh on youtube, and wanted to try it in nushell)
On the other hand, I always thought the next commit would fix everything, but it didn't. (@fdncred I am really sorry)
Of course, I also tried my best to adjust the structure of the code to make it easy to understand and have acceptable performance.
Logically, it is very common for comments and code to be out of line. Even I may be wrong. In the end, you still have to look at the code. I think that in the case of limited energy, it is better to spend energy on improving the code than writing documents.

@fj0r
Copy link
Contributor Author

fj0r commented Apr 29, 2023

  • new decorator type |> for head of left
    For rigth_prompt, applying powerline style is relatively easy, but left_prompt is much more complicated.
    Initial version may be buggy, just seems to work.
    So I refactored to make it more intuitive.
    Under the premise of avoiding unnecessary ansi escape code as much as possible, different processing methods are required for the head, tail, and middle (I thought it was enough to just deal with the middle and the tail before).

@fdncred
Copy link
Contributor

fdncred commented Apr 29, 2023

just to be clear, we don't require multiple paragraphs to detail what is changed. what we'd like to see is a high level description of what the PR is changing, what is going on. if there are breaking changes for users, we'd like a little more detail. we also want a proper pr title. e.g. this title was "refactor". that's too vague. i changed it to "refactor powerline script" which is probably good enough.

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.

4 participants