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

fisher with custom $fisher_path is not supported #51

Closed
legeana opened this issue May 15, 2023 · 3 comments · Fixed by #54
Closed

fisher with custom $fisher_path is not supported #51

legeana opened this issue May 15, 2023 · 3 comments · Fixed by #54
Labels
enhancement New feature or request

Comments

@legeana
Copy link

legeana commented May 15, 2023

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:

  1. Set $fisher_path to a custom path, e.g. ~/.local/share/fisher.
  2. Install fisher: curl ...
  3. Load $fisher_path/conf.d and add functions/completions to the appropriate paths in your own configuration as described in First-class support for $fisher_path jorgebucaran/fisher#640.
  4. 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
Copy link
Owner

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 joseluisq added enhancement New feature or request help wanted Extra attention is needed labels May 15, 2023
@joseluisq
Copy link
Owner

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 joseluisq linked a pull request Nov 25, 2023 that will close this issue
@joseluisq
Copy link
Owner

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

@joseluisq joseluisq removed the help wanted Extra attention is needed label Nov 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants