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 leuven-theme #85

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

Add leuven-theme #85

wants to merge 1 commit into from

Conversation

fniessen
Copy link

Add a new modeline which includes a VC flag and the current dictionary, among others.

@milkypostman
Copy link
Owner

can you tell me about the dictionary? what is that?

On Thu, Apr 23, 2015 at 2:24 PM, Fabrice Niessen notifications@github.com
wrote:

Add a new modeline which includes a VC flag and the current dictionary,

among others.

You can view, comment on, or merge this pull request online at:

#85
Commit Summary

  • Add leuven-theme

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#85.

@fniessen
Copy link
Author

Yep, It is the "language indicator" (2 first letters) for the current dictionary used by Ispell (derived from ispell-local-dictionary or ispell-dictionary). Quite interesting info to look at a glance when you write text or emails in multiple languages (French and English in my case).

@fniessen
Copy link
Author

  • If the buffer is read-only, the indicator will be "%%"
  • If there is no particular dictionary specified, the indicator will show "--"
  • Otherwise, it will display the first 2 letters of the dictionary ("fr" or "am" in my case, for "francais" or "american")

@milkypostman
Copy link
Owner

i'm a little worried about merging this given the introduction of so many new faces that seem to be fairly independent of anything else. i think something closer to #83 would work a bit better.

things like powerline-normal-face and powerline-modified-face seem OK but the dictionary one feels weird to me; it's so specialized.

@fniessen
Copy link
Author

fniessen commented May 4, 2015

The faces I introduce are there because they do represent some different things, and we like to see them highlighted in some way. There are 2 versions of them, for active and inactive states; so, yes, that's quickly more than just a couple of faces.

Regarding the dictionary, it's something very interesting when you work with multiple languages, like many of non-Americans must do (often write in English, the rest of the time in their own language).

Now, why extra faces? Because the more there are, the more everybody is happy: you don't want to highlight the dictionary indicator, simply customize the face -- you can, everything's in there.

If there were no faces, either you'd have to see all the items with similar colors, or accept default values wished by the author. Not fun, when I'm using someone's package.

@milkypostman
Copy link
Owner

could we call it something like powerline-ispell-dictionary or something?
I guess I agree with your point about the faces. It would be nice though
that we update the other themes to take advantage of them.

On Mon, May 4, 2015 at 11:18 AM Fabrice Niessen notifications@github.com
wrote:

The faces I introduce are there because they do represent some different
things, and we like to see them highlighted in some way. There are 2
versions of them, for active and inactive states; so, yes, that's quickly
more than just a couple of faces.

Regarding the dictionary, it's something very interesting when you work
with multiple languages, like many of non-Americans must do (often write in
English, the rest of the time in their own language).

Now, why extra faces? Because the more there are, the more everybody is
happy: you don't want to highlight the dictionary indicator, simply
customize the face -- you can, everything's in there.

If there were no faces, either you'd have to see all the items with
similar colors, or accept default values wished by the author. Not fun,
when I'm using someone's package.


Reply to this email directly or view it on GitHub
#85 (comment).

@fniessen
Copy link
Author

fniessen commented May 5, 2015

Regarding the names, yes, don't hesitate to change them to your liking, and/or for more coherency of the overall themes and package. My goal is simply that those elements do exist by default in your package, whatever the détails.

Thanks!

@fniessen
Copy link
Author

Just saw that this wasn't merged yet. Is there any problem doing so? Best regards.

Copy link
Owner

@milkypostman milkypostman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

really sorry this took me over two years. I understand this can be a turn off but if you're still interested i will promise to respond quicker.

'((((class color))
(:background "#FFA335" :foreground "black" :weight bold))
(t (:weight bold)))
"Face to fontify modified files."
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please update these comments to say that they are only relevant to the leuven theme?

"Face to fontify modified files."
:group 'powerline)

(defface powerline-normal-face
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please rename to 'unmodified' face.

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

Successfully merging this pull request may close these issues.

None yet

2 participants