Powerlevel9k: RFC: Split out segments into fpath functions

Created on 8 Jul 2017  路  14Comments  路  Source: Powerlevel9k/powerlevel9k

Proposal:

  • Put each segment method (currently prompt_foo) into the functions directory.
  • Any helper methods would be added to the same file and would be invisible to the shell.
  • The current contents of the functions directory would likewise be split up.
  • The functions directory would be added to the $fpath and autoloaded.

    • Alternative approach: We could create the autoload -X stubs ourselves and not pollute the user's fpath.

      zsh # Tested and it works! p9k-foobar() { local oldfpath=( "${(@)fpath}" ) local fpath=( "/path/to/installation/functions" "${(@)oldfpath}" ) autoload -X }

  • All (public) functions in the functions directory would be prefixed with something like p9k- (utility functions) and p9k-segment- (segments).

Reasons:

  • The current powerlevel9k.zsh-theme is very large. Splitting it up would make it easier to read.
  • This would make reuse easier, since you could find all the functions.
  • Functions could do things like change the emulation, reset all setopts (temporarily) to ensure portability, use local setopts and even use local traps!
  • We could add tests to verify each function is documented; We could even generate a web page from the documentation for developers. e.g. the first block of comments could be processed as markdown.
  • Testing would be easier, since each function could be tested separately.
  • Profiling would be easier, since each function could be profiled separately.
  • Functions wouldn't clutter up the user's shell if they aren't used (currently 63 total).
  • Functions could be compiled individually to boost performance; power users who have a whole-zshrc .zwc file would not be affected by this (see AUTOLOADING FUNCTIONS in the man pages).
enhancement next

All 14 comments

This is a more concrete description of splitting things out into fpath that I mentioned in #528. Once this is done, then it would be easier to make the widgets reusable for other prompts.

@docwhat - I really, really like this idea. @onaforeignshore has made a number of other suggestions, as well, about ways to make improvements along the same lines, and I think your proposal, here, is a great wrap-up of everything we have discussed in separate places.

The benefits are many, and the implementation is surprisingly straight-forward. I'm all for it. Based on @dritter's 馃憤 to your post, I'm guessing he is, as well.

Questions:

  1. @dritter - How will this affect your async work? I'd love to get this rolling in next, but I have to imagine that merging it into your async branch will be a nightmare.
  2. Is there anything we need to worry about regarding other ZSH frameworks? I feel like everytime we change something regarding how something is loaded, Antigen or Prezto break.
  3. I love the webpage documentation idea. @onaforeignshore did some work in #481 to create MD from comments. Tying this into gh-pages, I believe, is still a WIP, though.
  4. In some cases, our functions have a number of conditionals based on things like operating system. Would it ever make sense to dynamically map different OS-specific versions of functions to the same segment to simplify the function code? We would have more code overall, but it would be simpler. Frankly, I'm not sure how easy this would be, though.

@docwhat - Are you up for taking on this effort? If not, are you up for reviewing and testing the results?

I'm guessing he is, as well.

Yes I am! :)

And yes, it would impact the async branch pretty much. I would want to delay this a bit, until we get the async branch merged into next.

Is there anything we need to worry about regarding other ZSH frameworks? I feel like everytime we change something regarding how something is loaded, Antigen or Prezto break.

I think that #562 should fix those problems and show a helpful message when it does break. It has special case handling for Antigen < v2.1.0 and should work without problem in Antigen v2.1.0+.

In some cases, our functions have a number of conditionals based on things like operating system. Would it ever make sense to dynamically map different OS-specific versions of functions to the same segment to simplify the function code? We would have more code overall, but it would be simpler. Frankly, I'm not sure how easy this would be, though.

I think using per-os helper functions makes more sense than per-os functions. The idea is similar, but the OS bits can be varied per-segment without any extra work from the main loader.

Are you up for taking on this effort? If not, are you up for reviewing and testing the results?

As long as everyone can pair on it. :-D

We should collect some standards to normalize on...

  • All methods start with -p9k- or -powerlevel9k-. example:
    zsh -p9k-segment-dir { ... }
    Except for ones that must conform to an external API requirement (e.g. prompt_powerlevel9k_setup required by the prompt command).
  • setopt local_options is your friend.
  • Avoid external programs whenever possible; zshexpn(1) is your friend!
  • When in doubt, ask!

@docwhat - Agreed! Let's start some style guidelines in the Developer's Guide on the wiki, here: https://github.com/bhilburn/powerlevel9k/wiki/Developer's-Guide

I would like to use p9k everywhere possible (honestly, I regret making the config flags POWERLEVEL9K_*).

I'm in total agreement with everything else in your list =)

We should make the configuration an associative array, if we're changing everything. :-)

@docwhat 馃憤

Any update on the Developer's Guide in the Wiki?

We haven't actually done anything that we talked about in this issue, yet, hah. These are all things I _very_ much want to see done, I just haven't had the time to sit down and do them, yet.

I'm about to kick off a big list of major P9k improvements that need to happen - this issue will be an important part of them.

A lot of this is under work in #771, by the way! You can check out the branch where @onaforeignshore has put in a ton of work and @dritter is now helping here: https://github.com/bhilburn/powerlevel9k/tree/onaforeignshore-code_separation

Closing this one out, as it's basically been implemented in the next branch as part of the new codebase!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

xufab picture xufab  路  4Comments

mingrammer picture mingrammer  路  4Comments

dritter picture dritter  路  5Comments

jpdoria picture jpdoria  路  5Comments

gipert picture gipert  路  5Comments