-
-
Notifications
You must be signed in to change notification settings - Fork 40
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
Add auto-update functionality #270
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's quite a few places where I might've missed sticking to the same style/naming conventions as other places in znap. I'm happy to change this based on suggestions!
@@ -48,6 +48,11 @@ To run `znap pull` on specific repos only, including ones you have set to be exc | |||
% znap pull <repo> ... | |||
``` | |||
|
|||
To be prompted to automatically update with `znap pull` every 30 days, add the following to your `.zshrc` file: | |||
```sh | |||
zstyle ':znap:pull' auto-update 30 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm more than happy to take feedback on naming of the zstyle -- not sure what the most consistent with the other styles would be. This is different in that it doesn't have a repo name as part of it. Also not sure if this should include "interval" or "days" or something as part of the name.
Also, it could be possible to have styles on whether or not to prompt.
@@ -58,6 +58,10 @@ zstyle -T :znap: auto-compile && | |||
add-zsh-hook preexec ..znap.auto-compile | |||
add-zsh-hook zsh_directory_name ..znap.dirname | |||
|
|||
if zstyle -s ':znap:pull' auto-update _ ; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure whether it's better to test here or not. We have to test in the function, or pass the interval as a parameter (which just means setting a local here in init). My thinking was that we can avoid reading the file if the style is not set 🤷
It's also not clear from the issue whether a prompt like this is what's wanted, or rather integration with the myriad cron-like systems out there, which would be quite a lot of work. If what I've written isn't desired in znap, I won't be upset at all, happy to abandon it. I'd written it anyways for myself, since this is the functionality that I wanted on my machines. |
4dc13ad
to
8902235
Compare
Looking forward to have it merged |
I think it's a bad idea to check on each shell startup whether to update. I would rather solve it in way that's similar to how Are you interesting in updating your pull request to provide the initial framework for this? You don't need to implement support for all possible Thanks for contributing! 🙂 |
Fixes #102
Before submitting your PR (pull request), please
check the following:
similar to yours. If there is, please first
discuss over there.
Rules of a Great Commit
Message.
Fixes #<bug>
orResolves #<issue>
in itsbody (not subject) for each issue it resolves
(if any).
squashed
any redundant or insignificant commits.