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

Additional CHANGE_SUCCESS_CMD Functionality #164

Open
alex-courtis opened this issue Apr 15, 2024 · 5 comments · May be fixed by #168
Open

Additional CHANGE_SUCCESS_CMD Functionality #164

alex-courtis opened this issue Apr 15, 2024 · 5 comments · May be fixed by #168
Labels
feature New feature or request

Comments

@alex-courtis
Copy link
Owner

#162 adds CHANGE_SUCCESS_CMD as a success hook. Additional information coould be built on this.

Issue to discuss these @matthewwardrop

Possibilites include:

@alex-courtis alex-courtis added the feature New feature or request label Apr 15, 2024
@matthewwardrop
Copy link
Contributor

Hi @alex-courtis ; apologies for the delay (as usual life expects a lot from me!).

What kind of feedback would you like here? I really like the idea of state-change hooks, but it does seem like a missed opportunity to think more about both additional hooks (like failures) and multiple hooks for the same state change. I suspect we could run multiple commands in the existing implementation by stacking sh-commands... though I cannot remember (and don't have time to look into the code as I type this to check) whether multi-line strings are supported by the config parser [call me lazy, but I don't want to have to create another external script file to act as the hook].

I do also like the idea of a "last" state, but I'm not sure in practice how much value it would add. I don't think I would use it personally. I'm also not sure what I would do with a config reload hook (as distinct from the mode-changes effected by reloading the config), but perhaps there are good examples?

@alex-courtis
Copy link
Owner Author

alex-courtis commented Apr 20, 2024

Many thanks for your thoughts!

I suspect we could run multiple commands in the existing implementation by stacking sh-commands... though I cannot remember (and don't have time to look into the code as I type this to check) whether multi-line strings are supported by the config parser [call me lazy, but I don't want to have to create another external script file to act as the hook].

That is achievable. YAML does support multiline. Providing the command is expressed as a single shell executable e.g. notify-send foo ; notify-send bar it should function.

Script files in this sort of context are divisive. The user could use either, however a script could be encouraged as multiple commands could be difficult for the novice shell scripter.

I do also like the idea of a "last" state, but I'm not sure in practice how much value it would add.

Yes, I'm also not quite sure of a use case beyond some script action that depends on the delta.

I'm also not sure what I would do with a config reload hook

Unless this is completely trivial, YAGNI.

@alex-courtis
Copy link
Owner Author

@matthewwardrop
Copy link
Contributor

Love it!

@alex-courtis
Copy link
Owner Author

alex-courtis commented Apr 30, 2024

Work in progress: #168

Human readable change message is written to a file for notify-send to post.

20240430_173552

It might be time to remove all but the changed attribute from the "from:".

alex-courtis added a commit that referenced this issue Nov 9, 2024
alex-courtis added a commit that referenced this issue Nov 10, 2024
alex-courtis added a commit that referenced this issue Nov 18, 2024
alex-courtis added a commit that referenced this issue Dec 22, 2024
alex-courtis added a commit that referenced this issue Dec 22, 2024
alex-courtis added a commit that referenced this issue Dec 23, 2024
alex-courtis added a commit that referenced this issue Dec 23, 2024
alex-courtis added a commit that referenced this issue Dec 23, 2024
alex-courtis added a commit that referenced this issue Dec 23, 2024
alex-courtis added a commit that referenced this issue Dec 23, 2024
alex-courtis added a commit that referenced this issue Dec 23, 2024
alex-courtis added a commit that referenced this issue Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants