Skip to content
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

Add Powerline support to the tracking package. #214

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

travisbhartwell
Copy link

When the Powerline package is used, instead of modifying the
mode-line-format', the mode-line needs to be updated in global-mode-string'.

Updated the documentation for tracking-position to indicate this
behavior.

Fixes #137.

@travisbhartwell travisbhartwell force-pushed the fix-tracking-support-powerline-137 branch from c539c34 to 544c8f8 Compare August 18, 2015 16:27
@jorgenschaefer jorgenschaefer added this to the v2.1 milestone Aug 19, 2015
@jorgenschaefer
Copy link
Collaborator

Thank you for this contribution, very nice!

Please do not use (or <condition> <alternative>), that's highly unusual and reduces readability. I prefer (when (not <condition>) <alternative>). If you have a strong aversion against using not there, you can use (unless <condition> <alternative>), though I find unless generally pretty unreadable.

Also, is (featurep 'powerline) really the best way to check if powerline is enabled? It would return false positives when the powerline package is loaded but not used. A check like (memq 'mode-line-modes mode-line-format) might be more generic?

When the Powerline package is used, instead of modifying the
`mode-line-format', the mode-line needs to be updated in
`global-mode-string'.

Updated the documentation for tracking-position to indicate this
behavior.

Fixes emacs-circe#137.
@travisbhartwell travisbhartwell force-pushed the fix-tracking-support-powerline-137 branch from 544c8f8 to 052f5ca Compare August 19, 2015 14:57
@travisbhartwell
Copy link
Author

Thanks for the feedback.

I admit I was just copying the idiom I saw in timeclock.el and time.el, but I agree, using when is more readable.

I've force pushed a fix to my branch for that.

I'm still trying to determine the most reliable way to determine that powerline is active. I'll push that change when I figure it out and comment here so you can review that.

Thanks!

@travisbhartwell
Copy link
Author

I couldn't come up with a reliable, general way to detect if Powerline is active beyond the use of featurep, so I filed an issue, milkypostman/powerline#97 to get guidance on how to do this or how to make a change to Powerline to support this.

@jorgenschaefer
Copy link
Collaborator

Thanks for that! In the meantime, do you think adding a setting like powerline for tracking-position would be a good idea, and then adjusting the default value of tracking-position depending on that?

Also, (memq 'mode-line-modes mode-line-format) as a check might be sufficient already …

@bradens
Copy link

bradens commented Sep 10, 2015

Any progress on this one? I'd like to get using circe with powerline.

@travisbhartwell
Copy link
Author

@bradens Sorry, I've been ill and haven't had the energy to get back to this, I hope to soon (next day or so). I do welcome help to make the suggestions @jorgenschaefer mentioned.

@jorgenschaefer jorgenschaefer modified the milestones: v2.1, Backlog Nov 29, 2015
@theanalyst
Copy link

I would also like to see this feature in, @travisbhartwell will you be able to look into this?

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

Successfully merging this pull request may close these issues.

4 participants