Added feature that let's you specify how often to fetch

[hallzy]

I thought that this would be interesting, and useful for some people since 5 minutes may not be often enough for some people, so I made a modification so that you can easily customize how long to wait before fetching.

with --fetch_t you can specify how many seconds to wait before auto fetching.

using the --fetch option gives you the default of 5 minutes still.

"--fetch_t 45" for example would fetch every 45 seconds.

[hallzy]

Merged changes so that this could be merged easily

[michaeldfallen]

Rather than checking $1 you could use $@ (which contains all arguments passed to this script).

[hallzy]

Agreed. I will change to $@

[michaeldfallen]

Nice change, something I've been meaning to get in there for a while. I've left a few review comments that I think might be worth addressing. I might not be right on those, just starting a conversation.

If doing this I'd also like to follow the pattern the Colour and Format features have followed where they use prepare_bash_colors to set the default time to update, that way you can also use ENV VARS to configure in your ~/.bashrc.

[hallzy]

Thanks, I will look into these.

[hallzy]

I have made the changes recommended

Update: Looks like I may have an issue with the fetch now. Just wait before merging this for now.

Update on the update: It was an error in my parameter sub (that was new to me, and it turns out that I messed it up). All is good now

[hallzy]

Also, let me know your input on this, but I was thinking why bother even having --fetch_t ?? Maybe just use --fetch and check for a numeric argument following --fetch... if there is one, then use that... if not then use the default and then we wont need the error message that I put in... So basically expand the functionality of the already existing --fetch option.

I would like to hear your thoughts...

[hallzy]

I am not quite sure I understand what you mean by following the pattern of the colour and format features where they used prepare_bash_colors.

Are you thinking of having something like:

export FETCH_TIME='30'

Which we would be able to have in an rc file?

So instead of using the --fetch_t option, using something similar to the above command?

[michaeldfallen]

Looks good, thanks for addressing those.

On the pattern I mentioned it goes like this:

in prepare_bash_colours and prepare_zsh_colours create a new var:

FETCH_TIME="${GIT_RADAR_FETCH_TIME:-((5 * 60))}"

That will set FETCH_TIME to a default of 5 mins unless someone has set GIT_RADAR_FETCH_TIME in their .bashrc or .zshrc.

Then in the fetch function:

local fetch_time="${1:-$FETCH_TIME}"
...
if time_to_update $fetch_time; then
...

So if you pass a time to fetch like you do if they specify fetch_t then it uses that time, otherwise it falls back to the GIT_RADAR_FETCH_TIME and if that isn't set it falls back to the default 5 mins.

(Note: none of this code has been executed, likely there are typos everywhere)

[hallzy]

Ah... that makes sense... I will work on that now.

[hallzy]

Okay, I have it working now... Just doing a little bit of testing before pushing it... So with this addition, should we keep the --fetch_t or just get rid of it? Since now we can just do --fetch but have a variable in the bashrc or gitradarrc to specify how long to wait?

The commit I will push for this will get rid of fetch_t and just have fetch. Unless I here back before making the change... If you would prefer to keep the fetch_t, I can add it back afterwards.

[hallzy]

Changes are in.

Before you merge anything, I can go ahead and squash all of these commits into 1 as well if you would prefer? There has been a lot of changes that have been reverted etc... But if you want to keep the history that is cool too. Just let me know.

[hallzy]

Your PR is merged into this PR

[michaeldfallen]

👍 Thanks for this, good determination shown here. Merging, 🚢 it!