akirakyle / emacs-webkit

An Emacs Dynamic Module for WebKit, aka a fully fledged browser inside emacs
GNU General Public License v3.0
419 stars 24 forks source link

Display things in the modeline #15

Closed ymarco closed 3 years ago

ymarco commented 3 years ago

This PR displays the loading progress & normal/insert state in the modeline. 2 things I want to ask though:

  1. Should the state even be in the modeline? I'm kind of uncertain about this.
  2. Is it OK to use the unicode ↺ char for loading progress? I'm not sure how common it is font coverage -wise. It's easy enough for people to override webkit--display-progress, swapping it to something less pretty here upstream should be OK.
clemera commented 3 years ago

Is it OK to use the unicode ↺ char for loading progress? I'm not sure how common it is font coverage -wise.

You could test with char-displayable-p and choose to display another one if it is not available.

ymarco commented 3 years ago

That char doesn't work great in tty Emacs, at least not through my terminal, although ~char-displayable-p~ does return non-nil there.

I think I'll replace it with with with "loading:", its not that bad.

clemera commented 3 years ago

If you think it is worth it you could additionally use display-graphic-p to determine if you are running in a terminal.

ymarco commented 3 years ago

Replaced, now the loading string looks like "loading:90%" instead of "↺90%".

ymarco commented 3 years ago

If you think it is worth it you could additionally use display-graphic-p to determine if you are running in a terminal.

I don't think it's worth it, and users can always override that simple function easily if they want to complicate things, or know the char would always work on their setup. I'll leave it on "loading:".

akirakyle commented 3 years ago

Sorry for not addressing this PR sooner! I'm still trying to triage some of the major showstopper bugs, but this looks pretty good and something I was hoping to get around to anyways. I think I'll probably just merge the progress portion of this for now and add a recipe to the README for overriding the default "loading" text with an icon. I have a feeling the insert behavior will change at some point since I've not been super happy with how the webkit view still sometimes seems to refuses to give up focus. Once I settle that I'll revisit a mode line indicator since I do think it would be good to have by default. (The evil-collection recipe ties emacs-webkit insert to evil insert so I already get a visual indication)