-
Notifications
You must be signed in to change notification settings - Fork 71
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
Executing function on start #12
Comments
Hey @aristidesfl. I think this plugin needs some refactoring. First, the following functions:
Split them apart into separate files (file name and function name must match). Use a better prefix, like _done_FUNCTION_NAME, e.g, Then, the following functions:
Again, better to go with Make a directory mkdir conf.d
touch conf.d/done.fish
$EDITOR conf.d/done.fish Inside that file, add the definitions of _done_started and _done. Also initialize By the way, declare the scope of the variable and add a prefix too. set -g _done_initial_window_id "" I think that should do it. Let me know if something is missing or didn't work. |
Didn't know multiple function files were supported by fisherman. Also didn't know about conf.d. Two questions:
|
@aristidesfl
Reminds me that I need to write a fish plugin authorship handbook.
Good question. That's Because those functions register events, and the event emitter has no mechanism to determine whether a function in a file is a listener or not without sourcing the file first, so fish does nothing. Since we know those two are event listeners, we define them in the snippet.
For the same reason you should not use |
If we name the function according to the file where it is saved, they will be loaded and the event hooks will still work, no?
https:/jbucaran/fish-shell-cookbook/blame/master/README.md#L698 You mention aliases are not available for new shells, but that doesn't seem to be the case, unless you only set them for the login shell, right? |
No. The event emitter will fail (edit: not fail just won't do anything) to call the --on-event listener function because such function is not loaded in the session and will never be unless, of course, you source the file in which it is saved or invoke it interactively causing it to be lazy-loaded.
This means they do not persist.
|
But this is the way
I though the cookbook was talking about defining aliases in |
On a related topic, how does fisherman handle updated of scripts where the file names have changed. If I create new files with different names, will the old file stay installed when the user |
fish_prompt is special, because it's already available at the start of the shell. My advice applies to custom events or functions that are not loaded or those that you have no way to know whether they are loaded or not. Even if using fish_prompt, I would still use conf.d because it's the most appropriate, "fishy" way to do it. My guide does not talk about snippets yet, and it seems a lot of people don't know about them.
Me neither. The link just recommends you to not use then in your config.fish because it doesn't make sense and explains why. And yeah, I haven't had time to foolproof read the document, getting there tho hehe. |
But this will incur in slower initialisation like you mentioned with aliases correct? I'm only using native events in this case (
With snippets you mean the initialisation scripts in |
There's been a misunderstanding. Let me explain it one more time. See, this function: function _started --on-event fish_preexec
set _initial_window_id (_get_window_id)
end If you put that function in Why? Because it's not loaded in the session. See the screenshot above. Do you understand what's going on now? The only reason you see "HELLO" later on is because I forced the
Yup. That's the "official" name. It's derived from the same terminology used in some Linux distro or soemthing. |
This used to work, since that is how it was working until done v0.3. However now it doesn't seem to work anymore indeed. Could it be related to the fish version? |
Nope, it has always been like that. At least as far as I can remember since >=2.2. Perhaps you had the function loaded in your session and never realized 🤔 |
This doesn't make any sense. I just rolled back to v2.1.2 and indeed not working. But how do you explain all the people who installed the plugin and had it working? |
I can only speculate. If we think hard enough I'm sure we could come up with a hypothesis. Do you find it so odd? hehe EDIT: The function needs to be loaded only once. Perhaps they somehow loaded the function and experienced odd behavior from time to time, but never really paid too much attention. |
Moving forward, did you answer this question?
I'm thinking about putting the following in function __done_get_window_id
if type -q lsappinfo
lsappinfo info -only bundleID (lsappinfo front) | cut -d '"' -f4
else if type -q xprop
xprop -root 32x '\t$0' _NET_ACTIVE_WINDOW | cut -f 2
end
end
function __done_started --on-event fish_preexec
set -g __done_initial_window_id (__done_get_window_id)
end
function __done_ended --on-event fish_prompt
if test $CMD_DURATION
# Store duration of last command
set duration (echo "$CMD_DURATION 1000" | awk '{printf "%.3fs", $1 / $2}')
set notify_duration 10000
if begin
test $CMD_DURATION -gt $notify_duration # longer than notify_duration
and test $__done_initial_window_id != (__done_get_window_id) # terminal or window not in foreground
end
set -l title "Finished in $duration"
set -l message "$history[1]"
if type -q terminal-notifier # https:/julienXX/terminal-notifier
terminal-notifier -message "$message" -title "$title" -sender "$__done_initial_window_id"
else if type -q osascript # AppleScript
osascript -e "display notification \"$message\" with title \"$title\""
else if type -q notify-send # Linux notify-send
notify-send --icon=terminal --app-name=terminal "$title" "$message"
else # anything else
echo -e "\a" # bell sound
end
end
end
end
|
Yeah, I can see where you're coming from.
Good question. We do nothing, so old files will stay in the plugin directory. Maybe we should delete them. |
Yeah, because I remember specifically testing, when I renamed done to _done, there were conflicts. Even though I cannot reproduce it anymore, I'm afraid people will start getting multiple notifications. How would you implement something like that? Currently fisherman only copies files existing in master right? It doesn't even use tags to update? Have you considered requiring release tags? |
It does tags IIRC, or perhaps that's only fin. Can't remember right without looking at the code cause it's just not used so often. EDIT: I think it's in. |
Is fin a replacement for fisherman? What is the idea?
Fisherman seems to fetch master's HEAD even without tags, no? |
@aristidesfl fin is an another plugin I made after fisherman. The story is more complicated than that, but I'll spare you the details. fin has the cleanest and best design considering size and number of features I still managed to support. Fisherman handles more edge cases of course, supports oh-my-fish plugins more thoroughly and can query the remote org index. However, both download and install plugins concurrently.
For self-update yes. When you install a package I am sure support for tags was included/request by @colstrom IIRC. |
I just tested this once again, and |
@aristidesfl You need to specify the branch when installing, see the code here. |
@jbucaran I didn't intend to install specific tags, but prevent lose commits from being installed until a new release is created. The workaround it not committing directly to master. However using tags would enforce proper versioning of plugins. This could be interesting to show the user release notes and so on update. |
Thanks for the help! Version 1.0.0 uses a snippet instead of function files. |
Current version defines multiple functions which need to be loaded in order for the plugin to work. Fish however, only loads automatically the function which name matches the file's name (
_done
in this case).One option would be to define all the functions inside a main function, but then, the main function needs to also be executed when the shell is started, not only loaded.
@jbucaran Any idea on how to work around this?
Example:
The text was updated successfully, but these errors were encountered: