fisher with custom $fisher_path is not supported

legeana commented 1 year ago

Issue

gitnow assumes that fisher installs plugin into user's $__fish_config_dir. This is not always the case. If a $fisher_path is used, the plugin fails to load and produces a lot of error messages on fish startup.

How to reproduce:

Set $fisher_path to a custom path, e.g. ~/.local/share/fisher.
Install fisher: curl ...
Load $fisher_path/conf.d and add functions/completions to the appropriate paths in your own configuration as described in https://github.com/jorgebucaran/fisher/issues/640.

Try to install gitnow and observe the failure:

$ fisher install joseluisq/gitnow@2.11.0
fisher install version 4.4.3
Fetching https://api.github.com/repos/joseluisq/gitnow/tarball/2.11.0
Installing joseluisq/gitnow@2.11.0
       /home/liri/.local/share/fisher/functions/__gitnow_config_file.fish
       /home/liri/.local/share/fisher/functions/__gitnow_functions.fish
       /home/liri/.local/share/fisher/functions/__gitnow_manual.fish
       /home/liri/.local/share/fisher/conf.d/gitnow.fish
       /home/liri/.local/share/fisher/conf.d/gitnow_config.fish
       /home/liri/.local/share/fisher/completions/__gitnow_completions.fish
GitNow version  is installed and ready to use!
Just run the `gitnow` command if you want explore the API.
source: Error encountered while sourcing file '/home/liri/.config/fish/functions/__gitnow_functions.fish':
source: No such file or directory
source: Error encountered while sourcing file '/home/liri/.config/fish/functions/__gitnow_manual.fish':
source: No such file or directory
source: Error encountered while sourcing file '/home/liri/.config/fish/functions/__gitnow_config_file.fish':
source: No such file or directory
source: Error encountered while sourcing file '/home/liri/.config/fish/completions/__gitnow_completions.fish':
source: No such file or directory
Installed 1 plugin/s

I am pretty sure the issue is with gitnow_config.fish that tries to source fish functions directly. I am not sure why this is necessary.

Possible solutions

Check `$fisher_path` in gitnow_config.fish explicitly

Since $__fundle_plugins_dir is already queried in this file, checking $fisher_path could also be an option. This is how it looks on my PC after I install gitnow:

$ tree $fisher_path
/home/liri/.local/share/fisher
├── completions
│   ├── fisher.fish
│   └── __gitnow_completions.fish
├── conf.d
│   ├── gitnow_config.fish
│   └── gitnow.fish
├── functions
│   ├── fisher.fish
│   ├── __gitnow_config_file.fish
│   ├── __gitnow_functions.fish
│   └── __gitnow_manual.fish
└── themes

Make gitnow functions fish autoload-compliant

Don't explicitly source gitnow functions, and rely on $fish_function_path and other autoloaders to correctly load your plugin. For backward compatibility that can be done with something like set -q __gitnow_dont_autoload.

This will require splitting existing gitnow function files into multiple files, one file per gitnow function:

__gitnow_config_file.fish
- __gitnow_read_config.fish
- __gitnow_get_clip_program.fish
__gitnow_functions.fish
- __gitnow_new_branch_switch.fish
- __gitnow_slugify.fish
- __gitnow_clone_repo.fish
- __gitnow_clone_msg.fish
- __gitnow_check_if_branch_exist.fish
- __gitnow_clone_params.fish
- __gitnow_gitflow_branch.fish
- __gitnow_msg_not_valid_repository.fish
- __gitnow_current_branch_name.fish
- __gitnow_current_branch_list.fish
- __gitnow_current_remote.fish
- __gitnow_is_git_repository.fish
- __gitnow_has_uncommited_changes.fish
- __gitnow_get_latest_tag.fish
- __gitnow_get_tags_ordered.fish
- __gitnow_get_latest_semver_release_tag.fish
- __gitnow_increment_number.fish
- __gitnow_get_valid_semver_release_value.fish
- __gitnow_get_valid_semver_prerelease_value.fish
- __gitnow_is_number.fish

See how fish autoloads functions.

Move gitnow functions from `functions` to `conf.d` subdirectory

These functions will be loaded once by the plugin system, since conf.d is automatically sourced. And if these functions are ordered correctly, for example

conf.d/__10_gitnow_config_file.fish
conf.d/__10_gitnow_functions.fish
conf.d/__90_gitnow_config.fish -- make sure this file is sorted after functions it relies on

This approach is not idiomatic in fish, but it will load these functions correctly, and there is no more need to source functions/*.fish files.

joseluisq commented 1 year ago

Great ideas to improve Gitnow to support $fisher_path!

And yes, I'm in favor of being Fish autoload-compliant so any help more than welcome.

joseluisq commented 11 months ago

FYI I'm refactoring Gitnow to be Fish autoload-compliant on #54.

However, in order to support a custom $fisher_path, #54 should be finished first.

joseluisq commented 11 months ago

I already tested a default and custom $fisher_path Fisher installation and both work as expected. Please try out the latest changes.

joseluisq / gitnow