New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce flycheck-mode-line-lighter-base and use it, with test #880

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@tzz
Contributor

tzz commented Feb 17, 2016

Possibly remedies #879

Pretty trivial change, let me know if it's not OK in any way.

Edit (@lunaryorn): Connects to #879 (for waffle)

flycheck.el Outdated
@@ -3072,7 +3084,7 @@ nil."
"")))
(`interrupted "-")
(`suspicious "?"))))
(concat " FlyC" text)))
(concat " " flycheck-mode-line-lighter-base text)))

This comment has been minimized.

@cpitclaudel

cpitclaudel Feb 17, 2016

Member

Should we move the space to flycheck-mode-line-lighter-base as well?

This comment has been minimized.

@tzz

tzz Feb 17, 2016

Contributor

I think the space is always needed because Emacs doesn't add it automatically between mode lighters. But maybe some people want to save 1 space too? I'm not sure.

This comment has been minimized.

@lunaryorn

lunaryorn Feb 17, 2016

Contributor

I'd leave it as is and always add the space. The option is for the sake of simplicity, and should be fail-safe, in my opinion.

Those who'd like to remove the space as well should just customise flycheck-mode-line directly. If the space is that much pain you'll probably have gone to much greater length to shrink your mode line anyway, I think.

This comment has been minimized.

@cpitclaudel

cpitclaudel Feb 17, 2016

Member

Fine by me :) I'm just used to seeing either single-character lighters with no spaces, and multi-character lighters with a space.

@cpitclaudel

This comment has been minimized.

Member

cpitclaudel commented Feb 17, 2016

Looks good to me :) I made two small comments.

@tzz tzz force-pushed the tzz:master branch from 81aa20b to de0cb4e Feb 17, 2016

@tzz

This comment has been minimized.

Contributor

tzz commented Feb 17, 2016

I fixed the indentation, thanks. Let me know about the space in the lighter var.

flycheck.el Outdated
@@ -888,6 +888,18 @@ Set this variable to nil to disable the mode line completely."
:risky t
:package-version '(flycheck . "0.20"))
(defcustom flycheck-mode-line-lighter-base "FlyC"

This comment has been minimized.

@lunaryorn

lunaryorn Feb 17, 2016

Contributor

What about flycheck-mode-line-prefix? It's shorter, and that's what it is, isn't it?

This comment has been minimized.

@tzz

tzz Feb 17, 2016

Contributor

OK; renamed.

@tzz tzz force-pushed the tzz:master branch from de0cb4e to d73eb7c Feb 17, 2016

@lunaryorn lunaryorn closed this in ecf9d74 Feb 17, 2016

lunaryorn added a commit that referenced this pull request Feb 17, 2016

@lunaryorn

This comment has been minimized.

Contributor

lunaryorn commented Feb 17, 2016

@tzz Thank you very much. I applied your change and turned your test case into a buttercup spec, which is much nicer than ERT 😎

Thanks for this contribution, and please excuse my out-of-place comments in our discussion. 😊

@lunaryorn lunaryorn added the K-feature label Feb 17, 2016

@tzz

This comment has been minimized.

Contributor

tzz commented Feb 17, 2016

Ha ha you crazy kids with your buttercups... thanks, I'll learn how to use that for next time.

@lunaryorn

This comment has been minimized.

Contributor

lunaryorn commented Feb 17, 2016

@tzz SUCH BEAUTY, SUCH WOW 😎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment