Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reorganize plugins and library code #103

Closed
4 tasks done
derekstavis opened this issue Oct 5, 2015 · 6 comments
Closed
4 tasks done

Reorganize plugins and library code #103

derekstavis opened this issue Oct 5, 2015 · 6 comments

Comments

@derekstavis
Copy link
Member

We need a major overhaul in our directory structure. The omf plugin got inflated with stuff. Core library include unnecessary additions (mostly basename). Tackling plugins and themes auto-installable dependencies (#64) should fix most of our separation of concerns issues to allow us to divide most omf plugin functionality into separate plugins.

With this in mind I have designed and propose a functionality split using the following plugins:

  • core: Framework-wise functions
    • omf.core.reload: Reload Oh My Fish
    • omf.core.require: Loads a package
  • database: Oh My Fish package database functions
    • packages.installed: List installed packages
    • packages.available List packages available to install
    • package.install: Install a package
  • omf: Oh My Fish command line tool.
    • omf: Main utility function
    • omf.install: Install package or theme
    • omf.remove: Remove theme
    • omf. ...: other command functions
  • technicolor: Functions for colorising output, usable by both themes and plugins to color text.
    • tint:: Tint output text using colors
    • bold:: Make output text bold
  • vcs: A façade for version control, usable by themes which integrate VCS prompt.
    • omf.vcs.name: Outputs the VCS name (git, hg, svn)
    • omf.vcs.present: Check if in vcs
    • omf.vcs.branch: Print current branch name
    • omf.vcs.dirty: Check if repository is dirty
    • omf.vcs.staged: Check if repo has staged changes
    • omf.vcs.stashed: Check if repo has stashed changes
    • omf.vcs.touched: Check if repo has any changes
    • omf.vcs.status: Output characters for ahead/behind/diverged/detached/clean states

The implementation can be rolled out in parallel with the current structure, so no backwards-impacting changes would be done. The roadmap I see for implementing this structure follows the order below:

  • plugin-technicolor
    • I have some WIP work done in plugin-technicolor plugin
  • plugin-vcs
    • Written, need more testing
  • omf-cli
    • File naming adjustments
  • omf-core
    • Mostly copy-paste as-is and naming adjustments

Let me know what you think about this changes. Comments and improvements on structure are very welcome. I will keep this OP in sync with discussion below.

@bpinto
Copy link
Member

bpinto commented Oct 5, 2015

omf plugin is basically a package manager, or should be. But it's currently doing a little bit more as we have git functions (omf-vcs) that are not used by omf at all as they are helper functions for themes.

If we consider brew as a command, it's database already comes with it. You don't need to install a dependency to have a list of packages. So I wouldn't split omf in so many plugins, until we understand it better and are sure it's a good idea. If we correctly namespace the functions, it would be easy to split in the future.

Keep in mind that omf needs to load fast. The more plugins we have, the slower it is to load. It might be interesting to work on a omf cache feature that will cache both fish_function_path and fish_complete_path in the future.


  • omf: Package manager functions with namespaces
    • omf: Main utility function
    • omf.cli.install: Command line interface for omf.packages.install
    • omf.cli.remove: Command line interface for omf.packages.remove
    • ...
    • omf.packages.installed: List installed packages
    • omf.packages.available List packages available to install
    • omf.packages.install: Install a package
    • omf.packages.remove: Remove a package
    • ...
    • omf.bundle.list: List packages on a bundle file
    • omf.bundle.install: Install packages on a bundle file
    • ...
    • omf-out: Functions for colorising output, usable by both themes and plugins to color text.
      • ...
    • omf-vcs: A façade for version control, usable by themes which integrate VCS prompt.
      • ...

P.S.: reload and require would remain where they are for the moment.
P.S.: I am not fond of omf-out.
P.S.: I believe both out and vcs plugins don't need the omf prefix.

@bpinto bpinto added the idea label Oct 6, 2015
@bpinto
Copy link
Member

bpinto commented Nov 3, 2015

Hey @Pyppe do you think you could to work on this?

Update: Wrong issue I linked you to. This is the real one: #116

@derekstavis
Copy link
Member Author

Most of this was done in #207, but we need to fix upgrade mechanism to make sure newer code is used after updating the framework.

@oranja
Copy link
Contributor

oranja commented Dec 30, 2015

My prediction is that fixing the upgrade mechanism before merging #207 won't be enough.
What will happen, and I'd love to be wrong on this one, is that anyone who misses the first change will get both at the same pull and the upgrade function will still be the old one, meaning still no refresh between the pull and the package upgrades.
Somehow we must force the pull to stop at the commit that upgrades the update script.
Or, and you will want to throw my bones to the dogs for this: Keep one obsolete function, presumably one that is autoloaded but never used, and which is called in the course of updating the plugins, and hack it so it calls refresh and then starts the update process again. Perhaps we can somehow make a plugin do the same trick. Pull it, make it refresh on installation somehow, remove it in the end if it exists.
I admit that I didn't think any of this through yet. I should be doing the job I'm paid for, but I just had to stop and share my concerns...

@derekstavis
Copy link
Member Author

(...) anyone who misses the first change will get both at the same pull and the upgrade function will still be the old one, meaning still no refresh between the pull and the package upgrades.

You are right. We could roll them with some space in between (a month or two), but unfortunately it's hard to control the user updates frequency.

Somehow we must force the pull to stop at the commit that upgrades the update script.

Although a nice idea, this would have the same issue, we must be sure this is "pulled" before continuing.

Keep one obsolete function, presumably one that is autoloaded but never used, and which is called in the course of updating the plugins

And the winner is this idea! 🎉 By far it's the best solution.
Functions that was renamed must be kept for compatibility reasons in a compat directory.

@derekstavis
Copy link
Member Author

Done in #207

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

No branches or pull requests

3 participants