Install event support #573

joseluisq · 2020-06-25T08:02:39Z

Following the same aproach as the uninstall event, this PR adds support for a install event as well.
This event can be used following file_name_install event nomenclature.

Example:

function foobar_install -e foobar_install
  echo "Installing foobar..."
end

In this case, foobar_install function will get called when the foobar package gets installed via fisher add foobar.

I have also refactored the documentation under the "Configuration snippets" section.

it can be used following "file_name_install" event nomenclature function foobar_install -e foobar_install echo "Installing foobar..." end

jorgebucaran · 2020-06-25T09:11:10Z

Hi @joseluisq. What's the use case here?

joseluisq · 2020-06-25T09:28:52Z

What's the use case here?

@jorgebucaran when someone wants to perform a specific task once (during a package installation only).

For instance, I want to download a third-party file (E.g a config file than I might update on every release) but only when users install the latest package.

Actually I found no way to do this.

joseluisq · 2020-06-25T09:35:38Z

fisher.fish

+
+                set -l pkg_name (command basename $pkg .fish)
+
+                if test "$pkg_name" = "$current_pkg_name"


@jorgebucaran basically here I'm checking for emitting just once the event during installation.

Using your same uninstall approach on _fisher_rm function.

fisher/fisher.fish

Lines 399 to 401 in c2b3ab0

case $pkg/conf.d\*

test "$filename.fish" = "$target"; and emit "$filename"_uninstall

set target conf.d/$target

Will this actually run only once? What happens when I run fisher again? See #527

Will this actually run only once?

Yes, when I say once I mean on every fisher add foobar only.
Additionally, for example for new sessions created opening a new terminal tab, this event never gets emitted.

My concern was that I have no found a solution to make stuff on just every fisher add foobar.
Unless you have another more proper way to do it.

What happens when I run fisher again?

I have tried many times and I didn't see issues. I only saw that the uninstall event gets emitted at the beginning on the second attempt (related to #527), but then the normal install event gets emitted just at the end of the installation.

I have tested this PR with my package on local which has both events defined. Then performing fisher add $PWD many times as needed.

See #527

Is that already implemented or in progress?

PatrickF1 · 2020-07-12T04:39:43Z

Hi all,
As a package author, I am very interested in this as well. My use case will be to symlink a git-related function being installed by fisher into the user's bin, or at least print out a message advising the user to do so, so that the functions can be executed by git. e.g. if you put function called git-command in the user's path, then git command will execute git-command. Unfortunately, there's no clean way to do that right now.

@joseluisq if I may make a suggestion, I think a cleaner way to execute only on package install is to through an install.fish similar to how there is a uninstall.fish. It's so clean and simple thanks to the very elegant way @jorgebucaran designed fisher. See what I mean at https://github.com/jorgebucaran/fisher/blob/master/fisher.fish#L382.

jorgebucaran · 2020-07-12T05:07:22Z

@patrickf3139 "$pkg_name"_install.fish you mean, right?

PatrickF1 · 2020-07-12T05:23:13Z

I mean the file, not the function that is triggered by the uninstall event. Here is an example https://github.com/patrickf3139/open_github/blob/master/functions/uninstall.fish.

jorgebucaran · 2020-07-12T05:31:30Z

@patrickf3139 Using an event is probably better.

@joseluisq Would it be possible to achieve what you are trying to do using a conf.d snippet? Just want to make sure we're not reinventing the wheel here.

PatrickF1 · 2020-07-12T16:32:42Z

@jorgebucaran Would you mind explaining why an event in a conf script is better than a install.fish file?

PatrickF1 · 2020-07-13T21:09:37Z

It just seems to be that a file that only has to be read once when the plugin is installed vs a function that has to be re-read (though not executed most of the time) on every single fish invocation might be more elegant and efficient.

joseluisq · 2020-07-20T10:26:16Z

@joseluisq Would it be possible to achieve what you are trying to do using a conf.d snippet? Just want to make sure we're not reinventing the wheel here.

@jorgebucaran can you provide me an example on how to do so?

@patrickf3139 Using an event is probably better.

@jorgebucaran I also support the Fish event emit functionality.
@patrickf3139 an install.sh file can be an option too but Fish already provides event handlers so I support the idea of using an install/uninstall events.
You can easily source a install.sh file within a function if you want too.

PatrickF1 · 2020-07-20T15:57:12Z

I see, is the reason to use event handlers because fish already provides support for them, so it's less logic to have to include and maintain in fisher?
I still slightly prefer install.fish because it seems to me that conf.d is for configuration, whereas this is for installation, so it's poor organization of logic. However, I don't think it's a big deal and will defer the design decisions to Jorge. He has already made many brilliant, well thought out design decisions with fisher.
Thank you both!

IlanCosman · 2020-07-31T19:30:36Z

I would also have a use for this feature in Tide where I want to offer the user the configuration wizard on first install but not on every shell startup. One could do this using a conf.d snippet that removes itself, but that seems unnecessarily awkward.

jorgebucaran · 2020-08-01T16:29:36Z

@joseluisq Switching to main and deleting master closed this PR. My bad José! 😅

You may create a new PR against the repo's new default branch, but I'd probably hold that off for now. I'll look into this when I start working on #582, at which point I'll probably rewrite this part entirely. See that Install, Update, and Uninstall event support will be introduced in one form or another in Fisher 4. 🎉

PatrickF1 · 2020-08-01T16:51:00Z

Did you switch from master to main to make the language more inclusive? If so, 👏

jorgebucaran · 2020-08-02T01:29:54Z

Definitely!

joseluisq · 2020-08-02T19:54:55Z

@joseluisq Switching to main and deleting master closed this PR. My bad José!

You may create a new PR against the repo's new default branch, but I'd probably hold that off for now. I'll look into this when I start working on #582, at which point I'll probably rewrite this part entirely. See that Install, Update, and Uninstall event support will be introduced in one form or another in Fisher 4.

I'm wondering whether you just closed this because of branch renaming or some future v4, what happen with the stable v3?
Do I have to wait for a next major release in order to use this feature? If so, I don't support your aproach. The PR has been made for the current stable release.
Anyway it's your decision as maintainer. I don't question it.

However as a result of this approach, that I don't share, I'm going to stop resending any PR, leaving this to someone else if there is some interest on it.

`_update`. Starting with 4.0 plugin authors should be able to reliably know when their plugin is installed, updated, or uninstalled. Related #526, #527 #573. - Remove "some" OMF plugin support (it won't go away completely since more than less of OMF plugins actually work fine). This means fully deprecating `init.fish`, `uninstall.fish`, etc. So long, and thanks for all the fish! Related: #581 - No cache fallback, no plugin dependencies, no more private package hosts, and no more gitlab/bitbucket support. #464, #579 - Require fish 3.0, use newer fish features, e.g., use `wait` to implement concurrent downloads. - Rely less on external tools. No awk, no sed, no basename/dirname. Just mv, rm, cp, and mkdir. Deprecate `fishfile` in favor of `fish_plugins`. This new file works like the old fishfile, but without comment support. See #524.

- Introduce event system. #526, #527 #573. - Deprecate `init.fish`, `uninstall.fish`, etc. #581 - No cache fallback, no plugin dependencies, no more private package hosts, and no more gitlab/bitbucket support. #464, #579 - Require fish 3.0, use newer fish features, e.g., use `wait` to implement concurrent downloads. - Rely less on external tools. No awk, no sed, no basename/dirname. Just mv, rm, cp, and mkdir. - Deprecate `fishfile` in favor of `fish_plugins`. This new file works like the old fishfile, but without comment support. See #524.

- Introduce new event system. #526, #527 #573. - Deprecate `init.fish`, `uninstall.fish`, etc. #581 - No cache fallback, no plugin dependencies, no more private package hosts, and no more gitlab/bitbucket support. #464, #579 - Require fish 3.0, use newer fish features, e.g., use `wait` to implement concurrent downloads. - Rely less on external tools. No awk, no sed, no basename/dirname. Just mv, rm, cp, and mkdir. - Deprecate `fishfile` in favor of `fish_plugins`. This new file works like the old fishfile, but without comment support. See #524.

jorgebucaran · 2020-11-05T18:42:56Z

Sorry for the great delay, but this is now finally implemented in 4.x. 🎉

cc @joseluisq @patrickf3139

joseluisq added 2 commits June 25, 2020 09:24

feat: install event support

f8cd171

it can be used following "file_name_install" event nomenclature function foobar_install -e foobar_install echo "Installing foobar..." end

docs: add event handlers description

8aed207

joseluisq mentioned this pull request Jun 25, 2020

More consistent installation process joseluisq/gitnow#11

Merged

joseluisq commented Jun 25, 2020

View reviewed changes

jorgebucaran mentioned this pull request Jul 12, 2020

Add hook for an install.fish that is run only once when the plugin is installed #577

Closed

jorgebucaran mentioned this pull request Aug 1, 2020

RFC: Fisher 4.0 #582

Closed

jorgebucaran closed this Aug 1, 2020

joseluisq mentioned this pull request Sep 11, 2020

Add fisher set command for universal variables #591

Closed

jorgebucaran mentioned this pull request Nov 2, 2020

Fisher 4.0 #596

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Install event support #573

Install event support #573

joseluisq commented Jun 25, 2020

jorgebucaran commented Jun 25, 2020

joseluisq commented Jun 25, 2020

joseluisq Jun 25, 2020

jorgebucaran Jun 25, 2020

joseluisq Jun 25, 2020 •

edited

Loading

PatrickF1 commented Jul 12, 2020

jorgebucaran commented Jul 12, 2020

PatrickF1 commented Jul 12, 2020

jorgebucaran commented Jul 12, 2020

PatrickF1 commented Jul 12, 2020

PatrickF1 commented Jul 13, 2020

joseluisq commented Jul 20, 2020

PatrickF1 commented Jul 20, 2020

IlanCosman commented Jul 31, 2020

jorgebucaran commented Aug 1, 2020

PatrickF1 commented Aug 1, 2020

jorgebucaran commented Aug 2, 2020

joseluisq commented Aug 2, 2020

jorgebucaran commented Nov 5, 2020


		set -l pkg_name (command basename $pkg .fish)

		if test "$pkg_name" = "$current_pkg_name"

	case $pkg/conf.d\*
	test "$filename.fish" = "$target"; and emit "$filename"_uninstall
	set target conf.d/$target

Install event support #573

Install event support #573

Conversation

joseluisq commented Jun 25, 2020

jorgebucaran commented Jun 25, 2020

joseluisq commented Jun 25, 2020

joseluisq Jun 25, 2020

Choose a reason for hiding this comment

jorgebucaran Jun 25, 2020

Choose a reason for hiding this comment

joseluisq Jun 25, 2020 • edited Loading

Choose a reason for hiding this comment

PatrickF1 commented Jul 12, 2020

jorgebucaran commented Jul 12, 2020

PatrickF1 commented Jul 12, 2020

jorgebucaran commented Jul 12, 2020

PatrickF1 commented Jul 12, 2020

PatrickF1 commented Jul 13, 2020

joseluisq commented Jul 20, 2020

PatrickF1 commented Jul 20, 2020

IlanCosman commented Jul 31, 2020

jorgebucaran commented Aug 1, 2020

PatrickF1 commented Aug 1, 2020

jorgebucaran commented Aug 2, 2020

joseluisq commented Aug 2, 2020

jorgebucaran commented Nov 5, 2020

joseluisq Jun 25, 2020 •

edited

Loading