〈New〉import command. #288

ghost · 2015-01-04T03:15:31Z

Breakdown

Rewrites oh-my-fish.fish initial configuration and adds in-depth comments. The amount of code is reduced to ~10 lines of code.
Adds a new import command and introduces the idea of libraries. A library is category to conceptually organize cluster of .fish files such as plugins, themes and completions. A plugin intended to be used by other plugins and not by the user of the shell can be thought of as a library.
Simplifies _prepend_tree reducing amount of code to half.
import calls _prepend_tree for each plugin in the global $fish_plugins prepending subdirectories containing .fish files to the $fish_function_path.

Now it's possible to create complex directory trees with fish files organized according to the plugin's own API / structure. Paths containing .fish files will be added to the path automatically. This should encourage plugin authors to create a consistent project hierarchy without having to worry with load / configuration files modifying the $fish_function_path directly. .load files can still be used for more pertinent tasks such as downloading assets, declaring global variables, etc.

Note: _prepend_tree runs before .load files are sourced, so it is still possible to override any undesired behavior in the <plugin>.load file if necessary.

Notes

import replaces oh_my_fish.fish, _fish_add_plugin, _fish_add_completion, _fish_source_plugin_load_file functions inside functions/oh-my-fish.fish/, adds documentation and calls _prepend_tree, but it is still less code than the current configuration. This is possible using Fish {} brace expansion:

    set -l root $fish_path $fish_custom
    # ...
    set -l paths $root/$library $root/$library/completions
    set -l plugins     $paths[1..2]" fish_function_path"
    set -l completions $paths[3..4]" fish_complete_path"

    for path in $plugins $completions
      eval "_prepend_path "$path
    end

Performance

I tested adding al l the existing modules with a macbook pro and while I did notice a small lag when starting the shell the first time, the bottleneck seems to be simply adding so many plugins (and the fact Fish can not run Fish code in the background) and not _prepend_tree running for each plugin, conclusion I reached after commenting it and retesting without being able to notice any significant performance gains. See #96 for more info.

Usage

`import <path/library>[<path/library>..]`

Imports libraries, plugins, themes, completions. Prepends existing user custom/<library> directories to the path to allow users to override specific functions. Use import to load plugins, themes, handle plugin inter-dependency, etc.

Note: Adds only existing directories to the path.

Examples

import plugins/dpaste themes/bobthefish
import plugins/{cask,brew,django}

Extras

Also included in this commit is the msg plugin. See Add msg plugin #285.

Related & Other Issues

#286 #284 #285 #96

…mand.

…end_tree` to `require_tree`.

…Introduces the idea of libraries. Full comments.

…. Add detailed comments.

…epend_tree.

@light

+ Add more useful color generation. + Add @light and @dark constants to generate specific shades of colors. + Rename `msg._.` to `msg.util`. `util` is more readable and easier to type after a period.

+ Fix bug where a space was added after messages leading to unexpectedly erasing the first character if the last string passed to `msg` was a \r.

bpinto · 2015-01-06T17:01:14Z

Let's start the review... :)

ghost · 2015-01-06T17:02:11Z

Take your time and let me know anything.

bpinto · 2015-01-06T17:07:31Z

functions/_prepend_tree.fish

-#          Should appear at the end if used. Specifies the name of the
-#          global path variable to prepend the matched directories to.
-#          If not used, the $fish_function_path is assumed by default.
+#      [glob1 ] -a -and [glob2]


[glob1] [-o -or] [glob2] ...
[glob1 ] -a -and [glob2]

could share the same pattern.

[<glob> [<operator> <glob>..]] could possibly too.

I agree, however the reason for [-o -or], but only -a -and is that the or operator is optional (used by default if no operator is used), whereas and must be specified if you are planning to use it. Anyway I think your format is more like it so perhaps:

[<glob> -o -or <glob>..]

[<glob> [-a -and <glob>..]]

bpinto · 2015-01-06T17:57:23Z

I'll do some debugging, it's noticeable slower to start my terminal with this branch.

+ Removed unnecesary " . " for sourcing .load files. + Add comments to improve explaining the setup process. + Add more readable and user friendly if constructs as opposed to relying to any status code with `and` and `or`. + Remove `msg` plugin to split into another future PR. + Remove `rbenv` bug introduced in previous PR.

ghost · 2015-01-06T19:00:12Z

@bpinto Updated PR according to what was discussed.

Did you notice any improvements after this update? I am using bobthefish ATM (which is usually pretty heavy to load) to test and it seems to be running pretty smooth. Perhaps it was the evil eval "." who was causing some damage...

UPDATE: OK, I just tried the older version again, the one with eval "." and it's definitely slower than this update.

bpinto · 2015-01-06T19:20:17Z

Just tried, still not as fast. I believe the eval is causing it, let me try one thing.

…and use only for root paths (fish_path and custom_path). Do not add custom paths for completions, only for plugins and themes.

ghost · 2015-01-06T19:45:58Z

I got it. Now import is not using brace expansion, so we can get rid of 4 lines of code and say goodbye to eval, but we have two small, but rather similar for loops, not a big deal at all and it's actually better because I can select only plugins and themes for custom user configurations and not for completions which was rather extreme. The brace expansion trick was rather hard to read, smart, but just silly and possibly slow, plus it was requiring me to insert that pesky eval.

Now I am trying to fix a bug in _prepend_tree that only lets me traverse either the current directory or subdirectories.

…irectory or subdirectories instead of both.

bpinto · 2015-01-06T19:59:44Z

plugins/rbenv/rbenv.load

@@ -1,3 +1,8 @@
+set -l rbenv_dir "$ec"


There is still a bug on this code, don't worry about rbenv for now.

.oh-my-fish:prepend_path λ set rbenv_dir abc .oh-my-fish:prepend_path λ echo $rbenv_dir abc .oh-my-fish:prepend_path λ set rbenv_dir "$I_DONT_EXIST" .oh-my-fish:prepend_path λ echo $rbenv_dir

ghost · 2015-01-06T20:01:11Z

Just pushed the new changes. It seems to be loading just as fast as your master branch, which I am using side by side, checking out, starting a new console, checking out my import starting a new console, etc.

No more eval, the code is a bit shorter again and still we have import which is IMO much more elegant than _fish_add_plugin ...

bpinto · 2015-01-06T20:05:44Z

functions/_prepend_tree.fish

+    if contains -- $argv[1] -p --preview
+      printf "%s\n" $dir
+    else
+      _prepend_path $dir fish_function_path


https://github.com/bpinto/oh-my-fish/pull/290 alter this.

So, how do you proceed here? Do you merge your commit and then the import branch? Or do you want me to copy/modify anything?

I think it would be better as we can simplify the calls below. Otherwise I'd merge that #290 later.

Merged #290.

…-fish to use import.

….fish

…er customizations.

ghost · 2015-01-06T21:04:01Z

This merges your changes, and updates calls to the new _prepend_path. Running smooth here with a bunch of plugins + bobthefish. Do you think you can merge now? I would like to PR msg as well.

…m paths and resort to a simpler solution while sourcing .load files.

ghost · 2015-01-06T21:18:55Z

I added a small improvement during the initial setup.

# Prepend all user custom paths to the fish path and source load files. 
for custom_file in $fish_custom/**
  _prepend_path $custom_file -d fish_function_path
  switch $custom_file
    case \*.load
      . $custom_file
  end
end

In the same loop we are using to source the .load files we can also _prepend_path only those that are directories.

bpinto · 2015-01-07T01:30:01Z

I'll to another full review, but if you could squash and rebase, that would be the next step.

bpinto · 2015-01-07T01:39:39Z

functions/_prepend_tree.fish

-      eval $preview # skip when on preview mode
-        and printf "%s " $directory
-        or _prepend_path $directory $destination
+  # Travese $path and prepend only directories with matches.


Typo traverse.

…ix typos in comments.

… once and used declared variable consistently.

ghost · 2015-01-07T04:34:26Z

This is it. BTW now import prints matched directories by default, but you can skip this redirecting stdout. This seems the more unix-like approach I think. Everything should be working now. I will squash into one single nice commit and push again or create another PR.

Jorge Bucaran added 13 commits January 2, 2015 13:31

Tidy up _prepend_tree and rename to require_tree

e2c179b

Add require_plugin command.

568d9a2

Add msg plugin.

b1befc4

Rename _prepend_tree to require_tree and add require_plugin com…

3fb29c7

…mand.

Merge with 'require-plugin' to add require_plugin and rename `_prep…

f1e0cfe

…end_tree` to `require_tree`.

Remove informal msg._.test.

8989a1d

Tidy up comments.

6dcd3ec

Remove old require_plugin and require_tree.

358ae2e

Full _prepend_tree rewrite, cuts back +80 lines and simplifies code.

0b3031a

Add import command to handle adding plugins, themes and completions. …

a2b924b

…Introduces the idea of libraries. Full comments.

Greatly simplifies initial configuration using the new import command…

63a97c3

…. Add detailed comments.

Fix _prepend_tree stop using basename and losing path.

3f5b07c

Add examples and fix not using /Users/me/.oh-my-fish when calling _pr…

29ce8d0

…epend_tree.

ghost changed the title ~~〈New〉 import command.~~ 〈New〉import command. Jan 4, 2015

Jorge Bucaran added 5 commits January 4, 2015 12:57

Abort loading rbenv if no configuration file exists.

6492bb9

Rewrite some comments.

2206c32

Add new text separator to apply @color styles only to enclosed text.

16e8115

Update msg plugin.

e3aa8e8

+ Add more useful color generation. + Add @light and @dark constants to generate specific shades of colors. + Rename `msg._.` to `msg.util`. `util` is more readable and easier to type after a period.

Update msg #2

7c1fd43

+ Fix bug where a space was added after messages leading to unexpectedly erasing the first character if the last string passed to `msg` was a \r.

bpinto reviewed Jan 6, 2015
View reviewed changes

Remove variable expansion to generate plugin/theme/completion paths, …

7112c91

…and use only for root paths (fish_path and custom_path). Do not add custom paths for completions, only for plugins and themes.

Fix bug in _prepend_tree where it was traversing either the current d…

25e5518

…irectory or subdirectories instead of both.

bpinto reviewed Jan 6, 2015
View reviewed changes

Jorge Bucaran added 3 commits January 7, 2015 05:44

Update to new _prepend_path, complete import command and update oh-my…

a997b85

…-fish to use import.

Move _prepend_tree for user custom paths outside import in oh-my-fish…

74cfdbe

….fish

Move import call before prepending custom paths giving priority to us…

fb7bc8b

…er customizations.

Jorge Bucaran added 2 commits January 7, 2015 06:15

Remove using _prepend_tree in oh-my-fish.fish main setup to add custo…

784f5d5

…m paths and resort to a simpler solution while sourcing .load files.

Remove msg plugin added by mistake while preparing for a new PR.

365ac9f

Fix bug in _prepend_tree where a variable was commented out.

3e71c0f

bpinto reviewed Jan 7, 2015
View reviewed changes

Jorge Bucaran added 2 commits January 7, 2015 13:26

Fix wildcard bug in _prepend_tree, using an undefined in import and f…

e5b6f9a

…ix typos in comments.

Remove incorrect comment description, do away with variable used only…

9cfd6dc

… once and used declared variable consistently.

ghost closed this Jan 7, 2015

ghost mentioned this pull request Jan 7, 2015

[feat-import] + import command, ⁑ bad wildcard expansion bugfix in _prepend_tree. #291

Merged

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

〈New〉import command. #288

〈New〉import command. #288

ghost commented Jan 4, 2015

bpinto commented Jan 6, 2015

ghost commented Jan 6, 2015

bpinto Jan 6, 2015

ghost Jan 6, 2015

bpinto Jan 6, 2015

bpinto commented Jan 6, 2015

ghost commented Jan 6, 2015

bpinto commented Jan 6, 2015

ghost commented Jan 6, 2015

bpinto Jan 6, 2015

ghost commented Jan 6, 2015

bpinto Jan 6, 2015

ghost Jan 6, 2015

bpinto Jan 6, 2015

bpinto Jan 6, 2015

ghost commented Jan 6, 2015

ghost commented Jan 6, 2015

bpinto commented Jan 7, 2015

bpinto Jan 7, 2015

ghost Jan 7, 2015

ghost commented Jan 7, 2015

〈New〉import command. #288

〈New〉import command. #288

Conversation

ghost commented Jan 4, 2015

Breakdown

Notes

Performance

Usage

import <path/library>[<path/library>..]

Examples

Extras

Related & Other Issues

bpinto commented Jan 6, 2015

ghost commented Jan 6, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bpinto commented Jan 6, 2015

ghost commented Jan 6, 2015

bpinto commented Jan 6, 2015

ghost commented Jan 6, 2015

Choose a reason for hiding this comment

ghost commented Jan 6, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Jan 6, 2015

ghost commented Jan 6, 2015

bpinto commented Jan 7, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Jan 7, 2015

`import <path/library>[<path/library>..]`