Public ip segment #368
Public ip segment #368
Conversation
|
currently there's no default icon, i think it looks best without one but then I couldn't really find one that seemed appropriate, maybe with an appropriate one it would look better. |
|
I also tested killing the internet connection and it will still attempt to get a new IP at each timeout interval but it fails silently. This was the best behavior I could work out for not having a connection to the internet, input is very welcome with regards to how to handle that =] |
| [[ $timediff -gt '500' ]] && refresh_ip=true | ||
| # this will run the IP refresh with each new prompt while disconnected | ||
| # but will get a new IP immediately once reconnected rather than waiting | ||
| # for the timeout, not sure if this is ideal behavior or not |
There was a problem hiding this comment.
I think this is a good default behavior - let's roll with it.
There was a problem hiding this comment.
So I forgot to reread this section before pushing, I added a conditional to line 463 which makes this not behave exactly like this anymore. As it's written currently it will always create the tmp file if it doesn't exist and it will always check for a new ip if the tmp file is empty but it will no longer overwrite the tmp file with an empty result. If a fresh ip grab is attempted and none of the checks are successful then it will leave the tmp file alone. For example your connection to the internet dies, the segment will continue on doing nothing until the timeout is reached, at which point the prompt will attempt to grab a new IP every time the prompt segment is generated. I like this behavior but I think maybe we could add a touch to the file if the check is unsuccessful, that way it will still only check every X seconds even when your connection is down. What do you think?
There was a problem hiding this comment.
I'm good with that plan. I think the important behaviors are that:
- If the connection is lost, on the next update, the segment no longer displays an IP.
- If the connection comes back, the user does not need to manually kick the segment to bring the IP back.
What you described, above, achieves that. I think if you update the comments to reflect the actual behavior, we're good to go.
There was a problem hiding this comment.
Hmm, it should actually still display your last IP once you lose connection. I like that though. let me see what I can do to get that functionality in. I updated the comments already and added the behavior to touch the file to update it's modified time.
| fi | ||
|
|
||
| # write IP to tmp file | ||
| local public_ip=$(cat $POWERLEVEL9K_PUBLIC_IP_FILE) |
There was a problem hiding this comment.
I might be mis-reading this, but the comment on L466 and the code on L467 are confusing me a bit. Doesn't this code just readout the IP, not write it into the file?
There was a problem hiding this comment.
Yes this comment is supposed to be a few lines up. Will fix, sorry about that =]
|
Nice work, @rjorgenson! Very cool to see this get put together so quickly, and I think this will be really useful for our sysadmin users out there. I just left some questions in a small review =) |
…k into public_ip_segment
changed some offline behavior
|
@bhilburn I updated the offline behavior to the best solution I could get without removing the timeout feature altogether. If you are kicked offline it will continue to show your last known IP until the timeout expires, after that it will display nothing by default, but this can be configured by the user and continue to check for a new IP each time the segment is generated until it gets an IP and the timeout will reset. A user can set the timeout to 0 to have a real time polling with every new prompt of their public IP as well. Then they would notice as soon as they get disconnected. If this behavior sounds good to you then I'll write up a section in the README on it. I'm still not sure on an icon for this segment, or if it even needs one. Any input on that? |
|
@rjorgenson - The new changes look great, and the final scheme sounds good. Let's proceed with this. As for an icon, I just looked through all of the code points in Font Awesome and the standard UTF icons, and didn't see anything obvious. Go ahead and create the icon point in P9k, but we can leave it blank until we come up with something - I'm okay merging it without an icon. Let me know when the docs have been updated and it's ready for merge! @dritter - Any ideas? |
…k into public_ip_segment
|
Okay I've written up the README section. Writing isn't my strongest so if any of that could be changed to make more sense let me know =] I've also added the ability to specify a particular method and only ever attempt to use that method. This may save a bit of time if you know a particular method will always be available. |
fixed issue with POWERLEVEL9K_PUBLIC_IP_NONE being empty .. i hope
|
Whew, that was a lot of issues I noticed after my last comment =/ I think I got everything though .. i hope lol |
|
As this segment is related to the (internal)
But I am good to go with no icon as well. |
|
Hah! Thanks for the I'm so-so on our icon choices so far - to me, they all imply a wireless connection, which may not be the case. What do folks think about just using the |
|
Yes, I agree. These are not the best symbols, but there are no ethernet icons in awesome terminal fonts. ;) I am ambivalent with the |
|
@dritter - Great point. The Let's roll without an icon for now, @rjorgenson. |
|
No icon sounds good to me =] Just went over it again and pretty sure it's ready to merge. If anyone else wants to pull this PR and do some user testing in case I missed any bits that would probably be helpful =] |
|
Thanks for another great contribution, @rjorgenson! Excited to have this merged. |
@bhilburn mentioned in #365 that he would be interested in a public ip segment, i hacked one together for review/discussion on behavior/icons/etc. Currently it will only poll for a new IP every 5 minutes which is configurable by the user. It saves the IP to a tmp file also configurable by the user. It will attempt to use dig to get your IP if it's installed, if not it will use either curl or wget to poll an IP checker host in the same manner that neofetch uses. Let me know what you all think.