-
Notifications
You must be signed in to change notification settings - Fork 74
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
Highlight bibtex entry sections with faces. #375
base: master
Are you sure you want to change the base?
Conversation
CC @bdarcus See [[https://github.com/bdarcus/bibtex-actions][bibtex-actions]], the combination of faces and icons for different sections of the candidates offered by bibtex completion, gives a far better contrast between the different fields and aids the readability of the candidates.
Now when the new `bibtex-completion-format-always-show-symbols` variable is non-nil symbols will be shown whether the bibtex entry has the PDF or not. Faces can be used to distinguish whether they are present.
Interesting, thank you. I will have a closer look next week. |
I got really envious with how nice bibtex-actions looks so I decided to try and add some color to bibtex-completion. This may break compatibility with your package because it depends on Ideally I'd like it if you could remove the extra formatting logic from |
Just tested it out with helm and ivy, seems to be working fine 😄. |
@mohkale - cool! A few things.
First, I found it necessary/desirable to fork the format-entry function, as @tmalsburg didn't seem thrilled with the PR I submitted to revise that function to allow arbitrary templates. See #367. I would happily remove that function with that change though, and rely instead on Second, I'm not personally convinced I like a face for individual fields; at first glance it seems you've gone from no color to too much color ;-) But that's a first impression; need to think on it more. Affixation: currently, I'm only using that for the "prefix" column, to indicate presence of pdfs, links, notes. That shouldn't be an issue I'd think. Finally, another issue I've had, though maybe unrelated: I couldn't use Recent WIP enhancements to parsebib could provide an opportunity to simultaneously simplify and generalize I personally would like to keep |
But I guess just because a face is there doesn't mean it needs to be configured to use color? I do generally like my approach of the two different faces; main and suffix. But the two could be coupled. |
I did consider that being an issue... but then I thought it didn't really matter. The important thing is giving users the ability to customize those sections. If they don't like it they can customize the face to inherit from default (or whatever their default minibuffer face is). For example the title face is the default face and looks identicle to if that section wasn't propertised at all. If there's a general dislike of the abundance of color I'm okay with changing some of the default faces I've setup here to inherit from default as well. Should be a simple change so no worries for now 😃.
At some point bibtex-completion could do that as well (it's not like you can search on an icon anyways) but since affixation is still relatively new I'll leave that until it becomes a little more standard. |
I could do the same thing just using a third template. BTW, you can search for "has:pdf" etc in |
Interesting. I knew bibtex-actions had that (but don't know how it's implemented) but not bibtex-completion. Huh, I doubt I'll ever use it (narrowing seems more convenient) but that's interesting. |
In both cases, we just add the strings as hidden text. In |
The more you know 🏆. Incidentally I've also cleaned up my grouping function to now get the group directly from the bibtex entry, removing the need to have it in the candidate itself. What do you think about this? I'm not sure how I feel about making the year and author fields the same color :/. |
@mohkale -
Is this because the group function is independent of the candidates, just like affixation and annotation functions? E.g. you do need to hook it up to to
Looks good! I'd have to test it to know for sure how useful it is though. I don't tend to think a lot about types when searching/filtering, for example, so may not be so valuable to me personally. 🤷 If you think at some point it's worth considering for
Like you suggested above, probably strongly dictated by personal opinion. Could even be one changes one's own opinion over time! |
Kind of. I meant before I tied narrowing to grouping so that you had to list all the fields you can narrow on alongside the keys you narrow to them with: (defvar consult-bibtex-narrow+
'((?b . "Book")
(?u . "Unpublished")
(?l . "Booklet")
(?a . "Article")
(?m . "Misc")
(?o . "Other"))
"Narrowing configuration for `consult-bibtex+'.") If my bibtex file containined any type that isn't covered by the first 5 entries there it gets put into the Other group. And is shown with the title Other (which wasn't ideal). Now I just take the title from the bibtex entry directly and the narrowing config is used just for narrowing, not grouping. I did make one other small change, prefixing the candidates with the type (hidden of course) so you could filter by type without using narrowing. It's bound to come in useful I imagine. |
Another thought, @mohkale: As Titus has noted on the linked PR, using multiple templates wasn't the only way to achieve the current bibtex-actions UI. It was just what seemed most intuitive to me. Do you see a way to adapt this approach to also achieve that; where groups of fields are formatted differently? I don't know how flexible Emacs faces are ... |
PS - I saw this yesterday: |
There's actually discussion on emacs-devel about ditching affixation in favor of an enhanced annotation, or otherwise significantly changing affixation! To be clear, neither are really needed, particularly for the prefix part of the UI. Ideally, one could just put the symbols (including with icons) in the template at the beginning, and get the same visual result. |
I mean the obvious approach that comes to mind is to re-assign (let ((bibtex-completion-display-formats `((t . "foo"))))
(bibtex-completion-format-entry entry width)) But I assume you've already considered that. Nothing else really comes to mind. I saw it as well... but since I'm on a terminal they don't really mean much to me 😢. |
After thinking about this recently, @tmalsburg, we should probably follow his advice, and:
You could even just tag the faces, without defining them maybe, so that theme developers can then innovate on display? I can add more later, but WDYT? |
Hey, sorry for the slow response. I'm a bit hesitant to include new features. Reason is that the code of this project has organically grown over years and it getting more and more difficult to maintain. The truth is that this project could really use a thorough rewrite. Having said that, I'd be willing to accept an PR if it doesn't complicate things too much and if it works well for all possible configurations that we currently allow. The current PR has room for improvements in both areas.
(setq bibtex-completion-display-formats
'((article . "${author:30} ${title:*} ${journal:20} ${year:4} ${=has-pdf=:1}${=has-note=:1} ${=type=:3}")
(inbook . "${author:30} ${title:*} ${year:4} ${=has-pdf=:1}${=has-note=:1} ${=type=:3}")
(incollection . "${author:30} ${title:*} ${booktitle:20} ${year:4} ${=has-pdf=:1}${=has-note=:1} ${=type=:3}")
(inproceedings . "${author:30} ${title:*} ${booktitle:20} ${year:4} ${=has-pdf=:1}${=has-note=:1} ${=type=:3}")
(phdthesis . "${author:30} ${title:*} ${school:20} ${year:4} ${=has-pdf=:1}${=has-note=:1} ${=type=:3}")
(t . "${author:30} ${title:*} ${year:4} ${=has-pdf=:1}${=has-note=:1} ${=type=:3}"))) In sum, I do see the appeal of having colors (athough I wouldn't use them myself), but the current PR works well only for some users, not for others. For better or worse, helm/ivy-bibtex are highly configurable and this means that even the addition of benign-looking features can be surprisingly challenging. In this case, however, I think this complexity is manageable. |
A more high-level conclusion from this and related discussions: The Emacs ecosystem is in desperate need for a powerful and flexible layout engine for tables. Almost every helm and ivy project would benefit from that. Instead every project is coming up with its own half-baked solution. |
I may experiment with this a bit with my package. |
Actually, I'm thinking the obvious low-hanging fruit is author, title, date. With just that, I can reproduce, and improve, my UI. |
But proceedings use the editors field instead of authors. Wouldn't it look odd if authors were colored but editors in the same column not? |
So the gist of what you'd like changed is:
Both points make sense, although I worry the lack of any color may hinder discoverability about the feature. |
Sorry; yes, basically whatever goes in those "columns", including substitutions, get the same face. |
Unless you have another proposal, I think that's the most flexible solution.
I doesn't need to be empty, but we should choose defaults that can be assumed to work okay for most people. Something subtle. |
Per @tmalsburg, even if a user chooses colors, I would be extremely gentle with this, and leave it to designers, theme editors, and such to figure out the best aesthetics. The most important by far are authors and titles, for example. Those might get something subtle to distinguish from the rest, but I wouldn't necessarily add color per se to the rest. |
Specifically, the faces would need to work for people who have themes with dark text on light background and the reverse (night mode). Is that possible? |
I'm not really much of a designer so I doubt I'll be able to come up with such colors. For now I'll try to implement the alist stuff and leave the color defaults for later. |
I see the current PR inherits from font-lock-faces. Inheriting from something standard is a good idea but font-lock faces represent semantic categories that do not apply to authors, titles, ... Hm. |
I can probably get help on the face details. I asked on the doom discord. |
Inheriting from the font-lock is the correct approach imo, since it will yield good results with many themes. Many packages (including my own) use this approach. However we lose the semantic meaning, which is unfortunate. The other option is to create your own faces with colors for dark and bright themes. But then I recommend to use only a limited set of colors which hopefully works well with many themes. But this approach does not seem warranted here. It should only be used when you have special requirements, e.g., the Avy quick keys or mini-frame background, Corfu background are good examples where you want your own colors. Of course good themes should override these colors then. |
Here's info on a few transient faces, which inherit from font-lock. Face: transient-heading (sample) (customize this face)
Documentation:
Face used for headings.
Defined in ‘transient.el’.
Family: unspecified
Foundry: unspecified
Width: unspecified
Height: unspecified
Weight: unspecified
Slant: unspecified
Foreground: unspecified
DistantForeground: unspecified
Background: unspecified
Underline: unspecified
Overline: unspecified
Strike-through: unspecified
Box: unspecified
Inverse: unspecified
Stipple: unspecified
Font: unspecified
Fontset: unspecified
Extend: unspecified
Inherit: font-lock-keyword-face
Face: magit-keyword (sample) (customize this face)
Documentation:
Face for parts of commit messages inside brackets.
Defined in ‘magit.el’.
Family: unspecified
Foundry: unspecified
Width: unspecified
Height: unspecified
Weight: unspecified
Slant: unspecified
Foreground: unspecified
DistantForeground: unspecified
Background: unspecified
Underline: unspecified
Overline: unspecified
Strike-through: unspecified
Box: unspecified
Inverse: unspecified
Stipple: unspecified
Font: unspecified
Fontset: unspecified
Extend: unspecified
Inherit: font-lock-string-face |
Thank you @minad!
Does it matter much, given that defining the face itself provides semantics?
Seems maybe best approach initially is the former, and can iterate as needed? |
Now most of the extraneous bibtex entry faces from the previous commit are stored in bibtex-completion-field-faces as alist entries. The special faces for missing pdf/note symbols have been kept as stand-alone faces for now.
Could we have two main faces; maybe:
And then specific field faces could inherit from those? Could allow the sectional formatting, I think, I have in bibtex-actions. |
I've switched from multiple defface's to a single alist and pushed onto this PR. Atm the default faces are the same as before to make it easier for you to change them. Let me know if you have any issues. |
@mohkale This does not seem like the best idea, since it will make it impossible to tweak the faces in the themes. |
Not sure I understand but there are themes (e.g. the tango-plus-theme which I'm maintaining) where every color is consistently used to mean a thing (to the extent possible). If standard colors are then used for titles, authors, years, that basically breaks the theme. |
If it's not desirable I can revert it later. Have you seen the earlier comments from @tmalsburg regarding this. The gist is there needs to be someway for users to highlight fields that aren't standard. If need be we can take a hybrid approach later where we define explicit faces for the standard fields and add them to the alist (or use them in the formatting function directly) and let the user create custom faces for fields they themselves use. Edit: this comment specifically. |
I think @minad's proposal is to introduce some package-specific faces that inherit from font-lock faces and to use those in the configuration. This way, we get a functional look by default but themes can also customize the faces. |
I just tried this on bibtex-actions. Face definitions: (defface bibtex-actions
'((t :inherit font-lock-doc-face))
"Default Face for `bibtex-actions' candidates."
:group 'bibtex-actions)
(defface bibtex-actions-highlight
'((t :inherit font-lock-string-face))
"Face used to highlight content in `bibtex-actions' candidates."
:group 'bibtex-actions) Result: Notes:
|
CC @bdarcus
See bibtex-actions, the combination of faces and icons for different
sections of the candidates offered by bibtex completion, gives a far
better contrast between the different fields and aids the readability
of the candidates.
Screenshots
The proof is in the visuals.
Before
After