vermiculus / magithub

**DEPRECATED - please use Forge instead!** -- Magit-based interfaces to GitHub
GNU General Public License v3.0
579 stars 63 forks source link

Offline mode is still very confusing #304

Closed tarsius closed 6 years ago

tarsius commented 6 years ago
  1. The status buffer may contain a Magithub: OFFLINE... section. For that to be the case magithub-offline-p has to return non-nil.
  2. For magithub-offline-p to return non-nil magithub-settings-cache-behavior has to return t or refreshing-when-offline.
  3. Unless "cache overriding is in affect" (lets assume it is not), magithub-settings-cache-behavior returns a representation of the value of magithub.cache.
    • t for always
    • nil for never
    • when-present for whenPresent and undefined
  4. These values are documented as such:
    • always The cache will not be used, but it will be updated
      • That's really weird because "always" != "do not". Quite the opposite.
    • never The cache is never used. Data is always re-retrieved when online. When offline, there is simply no data.
    • whenPresent The cache is used if it exists. If there is nothing cached, data is retrieved when online. When offline, there is simply no data.
      • That last sentence doesn't appear to be true.
  5. I want to use "offline mode", so after having read these descriptions I have chosen whenPresent
    • because, while ambiguous, that comes closest to actually saying "use the cached value, if there is no cached value do NOT retrieve the value from the server".
    • I have NOT chosen always because its description very much says that the cached value won't be used, ever.
  6. However
    • (5)=always="do NOT use cache"(!) ===> (1)="offline"
    • (5)=whenPresent=`DO use cache" ===> (1)="online"
  7. When you are OFFLINE, then you want to use the cache.
    • And indeed Magithub seems to be doing that. The sections are shown and nothing is being retrieved.
    • However the documentation and the (absence of) the Magithub: OFFLINE section disagrees with that.

Please consider using these values instead (and don't map them to booleans, just use these values, as strings, throughou)t:

That last option seems fairly pointless, so I would suggest you replace this with a new variable:

And if you really want to make it possible to retrieve a value without caching it, you could re-add magithub.cache, but limit its purpose to what its name and to some extend its possible values always implied:


And if I am mistaken and the issue actually is that I fundamentally misunderstood something, then please consider that I have spend several hours today reading the source trying to understand what is going on. If that is so, then it would be clear that the documentation has to be massively improved and/or the implementation has to be made easier to understand.

tarsius commented 6 years ago

I found this useful:

  (defun magithub--debug ()
    (interactive)
    (message "\
magithub.cache                            %S
magithub-settings-cache-behavior-override %S
magithub-settings-cache-behavior          %S
magithub-offline-p                        %S"
             (magit-get "magithub.cache")
             magithub-settings-cache-behavior-override
             (magithub-settings-cache-behavior)
             (magithub-offline-p)))
tarsius commented 6 years ago

Just in case... I think Magithub is very nice in general! But the offline stuff seems very weird and the documentation needs a lot of love. And those issues have sadly kept me from really diving in until now.

vermiculus commented 6 years ago

Offline is weird. I'm not happy with it yet. Frankly the documentation needs love because I myself am not sure how it works, only that it can work.

rgrinberg commented 6 years ago

Wondering if there are any updates here. I'm still not sure how to configure magithub to behave as follows:

Do not make any network requests unless specifically asked to do so. I keep getting random delays when running magit-status and I don't know how to fix them.

vermiculus commented 6 years ago

I've been taking a hiatus from most of my projects to focus on wedding planning (two weeks left! 😱). This is something of a top priority for when I get back into it since it'll be one of the few times I can really look at it with fresh eyes.

@rgrinberg, it looks like the closest you can get is by setting magithub.cache=whenPresent. You can try also setting magithub.online=false, since it appears that used to exists, but it's no longer used in master for some reason. ~I'll have to look into why that disappeared and what was intended to replace it.~ It looks like this was removed by 7cbf387dc21c342cda338e87c98cba8c3bd777d2, though that commit did not provide a replacement.

But, your comment prompted me to think about this more, and I think I'll be taking the following approach using three boolean-valued options:

Here is a table of the behavior I'd like to get out of these options:

              | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7
make-requests | F | T | F | T | F | T | F | T
save-to-cache | F | F | T | T | F | F | T | T
use-cache     | F | F | F | F | T | T | T | T
  1. Magithub will not make requests and will effectively pretend the cache doesn't exist; this effectively turns the package off
  2. All data retreived and shown will be live data (new request every time, only caching within a single user command), but will not be saved to the cache
  3. Useless configuration (we'd save to the cache if we retreived data, but we'll never make requests)
  4. Like (1), but we'll save data to the cache
  5. We won't make new requests, but we'll use data from the cache if it's there.
  6. Like (4), but we'll make requests for the data we don't have in the cache. Oddly, we won't cache this data.
  7. Like (2), but at least we'll show cached data
  8. We'll use cached data when we have it, make requests when we don't, and save those responses for next time. This will be the default behavior.

Even when save-to-cache is nil, we'll still use a cache during a single command. In effect, we won't pull down the @octocat user for every issue they wrote in this repository just to get their username.

I'll review the original suggestions more closely – and compare to my first thoughts above – when life has calmed down 😄

vermiculus commented 6 years ago

Your proposed magithub.offline option is making me feel silly, though – I'm still going to see if there are any missed use-cases, but this seems like the simplest option right now. Even if there are missed use-cases, it's likely that magithub.offline will exist as a general setting and the more granular ones will override that behavior (offline=6, online=7) as overrides.

vermiculus commented 6 years ago

I remember now why the ability to never use the cache for retrieving data came about: one user (I'd ping if I could find the issue) was confused why refreshing the buffer didn't actually refresh data from GitHub. I obliged at the time, however, it did introduce a lot of complexity – for now, I think I'll go back to always using cached data if it exists as in magithub.offline=false (which will actually be implemented as magithub.online=true.

I'm hoping magithub-refresh (to be obsoleted and renamed magithub--refresh functionality has matured to the point that the original concern is no longer as dire – I recall it being even more confusing in the past.

vermiculus commented 6 years ago

Let me know if the above doesn't make things easier or if something is funky!

rgrinberg commented 6 years ago

Congratulations on the wedding.

I think you're on the right track here. May I suggest trying to use magithub in an environment with patchy internet connectivity, perhaps? 😆

vermiculus commented 6 years ago

@rgrinberg XD I tried something like that a while ago by installing a throttler – but I never got it working :/ unless you have a favorite by some random chance?