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

〈New〉import command. #288

Closed
wants to merge 29 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
e2c179b
Tidy up _prepend_tree and rename to `require_tree`
Jan 2, 2015
568d9a2
Add require_plugin command.
Jan 2, 2015
b1befc4
Add `msg` plugin.
Jan 2, 2015
3fb29c7
Rename `_prepend_tree` to `require_tree` and add `require_plugin` com…
Jan 2, 2015
f1e0cfe
Merge with 'require-plugin' to add `require_plugin` and rename `_prep…
Jan 2, 2015
8989a1d
Remove informal msg._.test.
Jan 3, 2015
6dcd3ec
Tidy up comments.
Jan 3, 2015
358ae2e
Remove old require_plugin and require_tree.
Jan 3, 2015
0b3031a
Full _prepend_tree rewrite, cuts back +80 lines and simplifies code.
Jan 3, 2015
a2b924b
Add import command to handle adding plugins, themes and completions. …
Jan 3, 2015
63a97c3
Greatly simplifies initial configuration using the new import command…
Jan 3, 2015
3f5b07c
Fix _prepend_tree stop using basename and losing path.
Jan 4, 2015
29ce8d0
Add examples and fix not using /Users/me/.oh-my-fish when calling _pr…
Jan 4, 2015
6492bb9
Abort loading rbenv if no configuration file exists.
Jan 4, 2015
2206c32
Rewrite some comments.
Jan 4, 2015
16e8115
Add new `text` separator to apply @color styles only to enclosed text.
Jan 4, 2015
e3aa8e8
Update msg plugin.
Jan 5, 2015
7c1fd43
Update msg #2
Jan 5, 2015
9c83f96
Fix bugs as discussed in github.com/bpinto/oh-my-fish/pull/288
Jan 6, 2015
7112c91
Remove variable expansion to generate plugin/theme/completion paths, …
Jan 6, 2015
25e5518
Fix bug in _prepend_tree where it was traversing either the current d…
Jan 6, 2015
a997b85
Update to new _prepend_path, complete import command and update oh-my…
Jan 6, 2015
74cfdbe
Move _prepend_tree for user custom paths outside import in oh-my-fish…
Jan 6, 2015
fb7bc8b
Move import call before prepending custom paths giving priority to us…
Jan 6, 2015
784f5d5
Remove using _prepend_tree in oh-my-fish.fish main setup to add custo…
Jan 6, 2015
365ac9f
Remove msg plugin added by mistake while preparing for a new PR.
Jan 6, 2015
3e71c0f
Fix bug in _prepend_tree where a variable was commented out.
Jan 6, 2015
e5b6f9a
Fix wildcard bug in _prepend_tree, using an undefined in import and …
Jan 7, 2015
9cfd6dc
Remove incorrect comment description, do away with variable used onl…
Jan 7, 2015
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 7 additions & 11 deletions functions/_prepend_path.fish
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,7 @@
# OPTIONS
# <path>
# Required. Specify the path to add to the list.
#
# [<glob> [<operator> <glob>..]]
# Glob pattern to match when traversing the path path.
#

# OPERATORS
# -d <DESTINATION PATH>
# Should appear at the end if used. Specifies the name of the
Expand All @@ -28,17 +25,16 @@
# _prepend_path $path -d $fish_function_path
# Add $path to $fish_function_path
#/

function _prepend_path
set -l destination_path PATH #$PATH is the default destination path
set -l len (count $argv)
# $PATH is the default destination path
set -l destination_path PATH
set -l path $argv

if test $len -gt 2
switch $argv[-2]
if test (count $argv) -gt 2
switch $path[-2]
case -d --destination
set destination_path $argv[-1]
set path $argv[1..-3]
set destination_path $path[-1]
set path $path[1..-3]
end
end

Expand Down
160 changes: 58 additions & 102 deletions functions/_prepend_tree.fish
Original file line number Diff line number Diff line change
@@ -1,141 +1,97 @@
#
# NAME
# _prepend_tree - add a dependency tree to a path
# _prepend_tree - add a dependency tree to fish_function_path
#
# SYNOPSIS
# _prepend_tree [-P --preview] <source path> [<glob>..]
# [-d --destination <destination path>]
# _prepend_tree [-p --preview] <path> [<glob>..]
#
# DESCRIPTION
# Traverse the source path and prepend each directory matching the
# glob list to the destination path or to the FISH function path.
# If no globs are specified, match any directories with `.fish`
# files. Return 1 if the source path is not specified.
# Search a path tree and prepend directories with fish files
# printing any matches by default. Use a glob list to include
# or exclude other file extensions. Use -p --preview to just
# print matches withouth modifying the path.
#
# OPTIONS
# -P --preview
# Use to skip adding anything to the path and echo all matched
# directories to stdout. Useful for debugging/testing.
#
# <SOURCE PATH>
# Required. Specify the path to find glob matches to prepend
# to the destination path.
#
# <glob>...
# Glob pattern to match when traversing the source path. If a
# directory is found, it is prepended to the destination path.
# `.fish` is assumed by default if not globs are specified.
#
# It is also possible to use logical OR / AND operators to list
# multiple globs. If none is used, OR is assumed by default.
# The OR operator
#
# The following operators are available:
# [-p --preview]
# Do not modify the path. Print directories that match the glob.
#
# ! -not
# Negates the following exression.
# <path>
# Required. Specify the path to search for glob patterns.
#
# ! glob1 glob2 → NOT glob1 or glob2
# [<glob> [<operator> <glob>..]]
# Glob pattern to match when traversing the path path.
#
# -o -or
# Any matches should meet at least one listed criteria.
# OPERATORS
# [! -not glob]
# Negates the following glob.
#
# glob1 -o glob2 → glob1 OR glob2
# [<glob> -o -or <glob>..]
# Default. Must meet at least one listed criteria.
#
# -a -and
# Any matches must meet all listed criteria.
#
# glob1 -a glob2 → glob1 AND glob2
#
# -d <DESTINATION PATH>
# 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.
# [<glob> [-a -and <glob>..]]
# Must meet *all* listed criteria.
#
# EXAMPLES
# _prepend_tree $lib_path
# Prepends all directories inside $lib_path containing `.fish`
# files to $fish_function_path.
# _prepend_tree $path
# Match directories in $path containing `.fish` files.
#
# _prepend_tree $lib_path -d PATH
# Prepends all directories inside $lib_path containing .fish
# files to a global variable named PATH.
# _prepend_tree $path \*.fish \*.sh
# Match directories in $path with either `.fish` OR `.sh` files.
#
# _prepend_tree $lib_path \*.fish \*.sh
# Prepends sub directories with either `.fish` OR `.sh` files.
#
# _prepend_tree $lib_path \*.css -a ! _\*.\* -d PATH
# Prepends sub directories that have `.css` extension, but do
# not start with `_`.
# _prepend_tree $path \*.fish -a ! _\*.\*
# Match directories with `.fish` files that do not start with `_`.
#
# AUTHORS
# Jorge Bucaran <jbucaran@me.com>
#
# SEE ALSO
# .oh-my-fish/functions/_prepend_path.fish
#
# v.0.1.0
# v.0.2.1
#/
function _prepend_tree -d "Load a dependency tree, matching `.fish` files by default."
set -l source
set -l destination fish_function_path
set -l glob
set -l depth 1
set -l len (count $argv)
set -l preview false

if [ $len -gt 0 ]
switch $argv[1]
case -P --preview
set preview true
set -e argv[1]
set len (count $argv)
end
end

[ $len -gt 0 ]
and set source $argv[1]
or return 1

if [ $len -gt 2 ]
# At least 3 arguments must exist
# to use the destination path.
switch $argv[-2]
case -d --destination
set destination $argv[-1]
end
function _prepend_tree -d "Add a dependency tree to the Fish path."
# Match directories with .fish files always.
set -l glob -name \*.fish
set -l path $argv[1]
if contains -- $path -p --preview
set path $argv[2]
end

if [ $len -gt 1 ]
set -l operator
for match in $argv[2..-1]
switch $match
# Parse glob options to create the main glob pattern.
if [ (count $argv) -gt 2 ]
set -l operator -o
for option in $argv[3..-1]
switch $option
case ! -not
set operator $operator !
case -o -or
set operator -o
case -a -and
set operator -a
case -d --destination
break # No more globs after this point.
case "*"
[ operator = ! ]
and set glob $operator $glob
or set glob $glob $operator
set glob $glob -name $match
set operator -o
if [ operator = ! ]
set glob $operator $glob
else
set glob $glob $operator
end
set glob $glob -name $option
set operator -o # Default
end
end
end

[ -z "$glob" ]
and set glob -name \*.fish

for directory in $source/**/
if not [ -z (find $directory $glob -maxdepth $depth | head -1) ]
eval $preview # skip when on preview mode
and printf "%s " $directory
or _prepend_path $directory $destination
# Null wildcard expansion will break the for loop even if $path is valid.
# $subs will become an empty list for directories without sub directories
# which is safe to use in the loop.
set -l subs $path/**/

# Traverse $path and $subs prepending only directories with matches.
for dir in $path $subs
# Use head to retrieve at least the first match.
if [ -z (find $dir $glob -maxdepth 1 | head -1) ]
continue
end
printf "%s" $dir
if not contains -- $argv[1] -p --preview
_prepend_path $dir -d fish_function_path
end
end
end
44 changes: 44 additions & 0 deletions functions/import.fish
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# NAME
# import - load libraries, plugins, themes, etc.
#
# SYNOPSIS
# import <path/library>[<path/library>..]
#
# DESCRIPTION
# Import libraries, plugins, themes, completions. Prepend existing
# user custom/<library> directories to the path to allow users to
# override specific functions in themes/plugins.
#
# NOTES
# $fish_path and $fish_custom point to oh-my-fish home and the user
# dotfiles folder respectively. Both globals are usually configured
# in ~/.config/fish/config.fish
#
# EXAMPLES
# import plugins/dpaste themes/bobthefish
# import plugins/{cask,brew,django}
#
# AUTHORS
# Jorge Bucaran <jbucaran@me.com>
#
# SEE ALSO
# functions/_prepend_path.fish
# functions/_prepend_tree.fish
#
# v.0.2.1
#/
function import -d "Load libraries, plugins, themes, etc."
for library in $argv
# Prepend plugins, themes and completions, traversing library
# trees and prepending directories with fish code.
_prepend_tree $fish_path/$library >/dev/null
_prepend_path $fish_path/$library/completions -d fish_complete_path

# Source each plugin, theme, etc., configuration load file.
for path in $fish_path/$library/(basename $library).load
if [ -e $path ]
. $path
end
end
end
end
78 changes: 19 additions & 59 deletions oh-my-fish.fish
Original file line number Diff line number Diff line change
@@ -1,74 +1,34 @@
###
# Helper functions
###

function _fish_add_plugin
set -l plugin $argv[1]
set -l plugin_path "plugins/$plugin"

_prepend_path $fish_path/$plugin_path -d fish_function_path
_prepend_path $fish_custom/$plugin_path -d fish_function_path
end

function _fish_add_completion
set -l plugin $argv[1]
set -l completion_path "plugins/$plugin/completions"

_prepend_path $fish_path/$completion_path -d fish_complete_path
_prepend_path $fish_custom/$completion_path -d fish_complete_path
end

function _fish_source_plugin_load_file
set -l plugin $argv[1]
set -l load_file_path "plugins/$plugin/$plugin.load"

if test -e $fish_path/$load_file_path
. $fish_path/$load_file_path
end

if test -e $fish_custom/$load_file_path
. $fish_custom/$load_file_path
end
end

function _fish_load_theme
_prepend_path $fish_path/themes/$fish_theme -d fish_function_path
_prepend_path $fish_custom/themes/$fish_theme -d fish_function_path
end

###
# Configuration
###

# Set fish_custom to the path where your custom config files
# and plugins exists, or else we will use the default custom.
# and plugins exist, or use the default custom instead.
if not set -q fish_custom
set -g fish_custom $fish_path/custom
set -g fish_custom $fish_path/custom
end

# Extracting user's functions – will be added later.
# Extract user defined functions from path and prepend later to
# avoid collisions with oh-my-fish internal functions and allow
# users to override/customize plugins, themes, etc.
set user_function_path $fish_function_path[1]
set -e fish_function_path[1]

# Add all functions
# Add functions defined in oh-my-fish/functions to path.
if not contains $fish_path/functions/ $fish_function_path
set fish_function_path $fish_path/functions/ $fish_function_path
end

# Add all defined plugins
for plugin in $fish_plugins
_fish_add_plugin $plugin
_fish_add_completion $plugin
_fish_source_plugin_load_file $plugin
end

# Load user defined theme
_fish_load_theme $fish_theme
# Add required plugins, completions and themes. Imported commands can be
# customized via the $fish_path/custom directory. To customize a theme,
# create a directory under $fish_path/custom/themes with the same name
# as the theme. Use the same approach for plugins, etc.
import plugins/$fish_plugins themes/$fish_theme

# Source all files inside custom folder
for config_file in $fish_custom/*.load
. $config_file
# Prepend all user custom paths to the fish path and source load files.
for custom_file in $fish_custom/**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why custom folder is handled differently. I could add a dependency from a custom plugin to another custom plugin.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By not using the same handler (import function) we are also not filtering the custom plugins using the fish_plugins env var

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$fish_path vs $fish_custom

It is handled different because the path we are prepending to $fish_function_path is not $fish_path but $fish_custom.

for custom_file in $fish_custom/**
  _prepend_path $custom_file -d fish_function_path
  switch $custom_file
    case \*.load
      . $custom_file
  end
end

We don't need to call import for custom because we are already prepending everything in custom to the path and sourcing all .load files by default.

By not using the same handler (import function) we are also not filtering the custom plugins using the fish_plugins env var.

$fish_plugins is just a list with plugin names that exist in ~/$fish_path_plugins. You can not add custom plugins via $fish_path. Those you put in /custom. Of course, plugins not under source control inside /plugins will always load anyway, but that's not what we are talking about here I presume.

Wildcards

I just fixed a bug in _prepend_tree where the wildcard was not correctly expanding and not running the for loop at all. See here also fish-shell/issues/683

Also and just for the record, it's good to know to have your wildcards clear:

  • $fish_custom/** Traverses both files and directories/subdirectories. Everything.
  • $fish_custom/**/ Traverses only directories/subdirectories.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to call import for custom because we are already prepending everything in custom to the path and sourcing all .load files by default.

This is not the existing behaviour. Custom plugins are just like builtin plugins, they are loaded only if you have them defined on $fish_plugins.

There is no reason to not add $fish_custom to import, we are probably going to do something similar in the future with another path once we add a vundle like system.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's recapitulate, there is $fish_path (usually /.oh-my-fish) and $fish_custom (usually /.oh-my-fish/custom), these two are paths.

There is also a global list of enabled plugins $fish_plugins. Plugins (the plugin name) you add to $fish_plugins will be handled by import during the initial setup. Plugins as well any other user functions (.load files too) defined in the custom will be added to the path and sourced respectively.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#Only plugins added to $fish_plugins are loaded.
for plugin in $fish_plugins
  _fish_add_plugin $plugin
  _fish_add_completion $plugin
  _fish_source_plugin_load_file $plugin
end

function _fish_add_plugin
  set -l plugin $argv[1]
  set -l plugin_path "plugins/$plugin"

# It loads the plugin from ~/.oh-my-fish
  _prepend_path $fish_path/$plugin_path -d fish_function_path
# It loads the plugin from ~/.oh-my-fish/custom
  _prepend_path $fish_custom/$plugin_path -d fish_function_path
end
# All files with .load inside custom folder are automatically loaded.
for config_file in $fish_custom/*.load
  . $config_file
end

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you mean, but the current implementation does not alter this behavior:

for custom_file in $fish_custom/**
  _prepend_path $custom_file -d fish_function_path
  switch $custom_file
    case \*.load
      . $custom_file
  end
end

Adds all user custom plugins, functions, etc., as well as sourcing .load configuration files.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are loading all custom plugin now, if they were defined on $fish_plugins or not.

This loop will go through each file and call _prepend_path on it. But this function should only be called for directories. So we will be calling this function many times unnecessarily (for each file).

Only .load files under $fish_custom root need to be searched and loaded. And the rest behaves just like the import function.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's true. Give me a few minutes.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's continue the conversation over the new PR discussion area. This PR is closed.

_prepend_path $custom_file -d fish_function_path
switch $custom_file
case \*.load
. $custom_file
end
end

# Re-adding user's functions so they have the highest priority
# Prepend extracted user functions so they have the highest priority.
set fish_function_path $user_function_path $fish_function_path
5 changes: 5 additions & 0 deletions plugins/rbenv/rbenv.load
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
set -l rbenv_dir "$ec"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another pull request?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😀 It was crashing my terminal when I tried loading the plugin so I figured it is loosely related, considering this PR modifies a lot of the foundational elements in the framework.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will break for other people, I don't have $ec defined on my computer for instance. So, I guess this would not work for me.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't use rbenv so I don't mind ignoring this, but I don't understand why it would break for other people. It will only not break for people without rbenv, like me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would this be executed if you don't have rbenv? I assume you don't have it set on your $fish_plugins.

That line is using my exported $RBENV_ROOT environment variable to define the rbenv dir. If we remove that, then it won't work for my customized install dir.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was crashing my terminal when I try to load the plugin while not having rbenv. Stupid thing to do anyways.

However I see I actually introduced a sloppy bug it seems.

What about moving it to the top:

if not [ -e "$HOME/.rbenv" ]
  exit
end

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know what happen to the other plugins if you don't have the dependencies installed.
I think this is fine. We could improve it later.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the only one giving trouble in my machine, but you are right, we could improve this later. Will remove in the next import PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

if [ ! $rbenv_dir ]
set rbenv_dir $HOME/.rbenv
end

set -l rbenv_dir "$RBENV_ROOT"
if [ ! $rbenv_dir ]
set rbenv_dir $HOME/.rbenv
Expand Down