RyanMillerC / poke-line

Minor Emacs mode to show buffer position using a Pokémon!
GNU General Public License v3.0
34 stars 2 forks source link

Fix: only check for png support when requiring #14

Closed progfolio closed 4 years ago

progfolio commented 4 years ago

Do we need to check for PNG support every time poke-line-create is invoked? I've moved the check to the initialization of the minor mode. If it fails, a user error is thrown. I don't have an Emacs on hand that doesn't have PNG support to test this, but I don't think this should hurt anything.

RyanMillerC commented 4 years ago

Good idea, we don't need to check every time. We could also print a message if PNG support is disabled.

The behavior now if PNG support is not available is to show a text bar with vertical lines showing position. For example, if I loaded Emacs into my terminal with emacs -nw and enabled poke-line-mode, my progress bar would look like: ||||||||||||||||||||||||||||----. Calling user-error would break that behavior.

progfolio commented 4 years ago

my progress bar would look like: ||||||||||||||||||||||||||||----. Calling user-error would break that behavior.

IMO the pipe char progress bar isn't desirable in this case. It's not a poke-line if it ain't got pokemon. If no png support is available why load the package at all?

RyanMillerC commented 4 years ago

The plain text progress bar was implemented in nyan-mode so I left when I forked that repo. I see your point. If you fix conflicts I will merge.

progfolio commented 4 years ago

The check is now at the head of the code section, so if the user's Emacs doesn't have png support, the feature won't be loaded. Tested by advising image-type-available-p like so:

(defun override (&rest args) nil)
(advice-add 'image-type-available-p :override #'override)

Which seems to have the desired effect.

RyanMillerC commented 4 years ago

PNG support is still being checked every render on line 146.

progfolio commented 4 years ago

Ryan Miller notifications@github.com writes:

PNG support is still being checked every render on line 146.

Apologies. Taken care of in latest commit.

RyanMillerC commented 4 years ago

Looks good, thanks for the update!

Quick note: I didn't fully understand the original code in my comment above. I thought it was only printing | and - characters if PNG was not available. It turns out that those are fallback characters that are replaced with the image (similar to the alt attribute on an HTML <img /> tag if an image can't be rendered). Mode-line in a terminal with poke-line enabled still shows a progress bar.