GNOME Bugzilla – Bug 701364
Add font scaling support for headings/titles
Last modified: 2014-01-30 18:33:06 UTC
This is a crosspost from the mailinglist-mail (following recommendation to post here instead). out of envy for the Emacs Latex-Plugin I implemented the feature to scale certain parts of a file accoring to the style definition. Specifically, this means that \chapter{}, \section{}, etc commands in tex files and <h1>, <h2>, etc commands in html files will have their contents presented in a larger form than the rest. Please finde the code for pull at: https://github.com/anduchs/gtksourceview-headlining/tree/headliningsupport (or in the attached patch-files against current head) Open Issues: - in <h1 class="test"> tags the attributes are note highlighted correctly anymore as it seems no context-matching is possible within a start-subpattern. Attached are also two illustrative test files along the patchset.
Created attachment 245736 [details] Test file latex
Created attachment 245737 [details] Test file html
Created attachment 245738 [details] [review] Patch to add support for scale property in schemes
Created attachment 245739 [details] [review] Patch to use scale property from styles files
Created attachment 245740 [details] [review] Patch to use headings from latex langspec
Created attachment 245741 [details] [review] Patch to use headings from html langspec
Hi Andreas, thanks a lot for the detailed report and patch and sorry for the lack of prompt feedback. I am not opposed to adding the scale property to the style tags and your patch looks good at a quick glance. However I do not think we want this by default on the styes schemes we ship by default, I think it would be surprising for many of our users which expect a classic fixed font text editor. Right now I am leaning toward merging your work (lang files patches included) but not the style scheme changes, asking people who want this feature to use a custom style scheme... I wonder however if there is a more friendly way to let the user opt in to this feature
Review of attachment 245741 [details] [review]: ::: data/language-specs/html.lang @@ -163,2 +169,5 @@ </context> + <context id="headings"> + <include> + <context id="title" style-inside="true" style-ref="title" end-at-line-end="true"> I do not think html headings should use end-at-line-end, no?
Hi Paolo, thanks for noticing at all... Regarding html.lang: I added this after some usage, because otherwise what happens is: User adds a H1 somewhere in the text, and all of a sudden, the whole document (up to the end) will turn big. Since multiline headings are rare (for html as well as tex) that's the reason I set this. Is there maybe the possibility to have a rule match, only if <end> appears in the text ? Also could maybe the braces-matching algos be added and used in styles ? Regarding the first part, I'm thinking of 3 possible solutions: 1. Have some Option in the Preferences. -> Undecided 2. Have an extra theme. -> Maybe we could introduce theme-inheritance; i.e. Classic.xml vs ClassicSized.xml (with <include style="Classic.xml" />) or similar. 3. Introduce config-options to style-files that can be set/unset via style-preferences. What do you say ? Best regards, Andreas
It is planned to have CSS color schemes. With this, we can dynamically override a color scheme to enable bigger fonts for headings.
I just wanted to come back to this now that 3.10 is out the door. I'm getting a little tired of maintaining this patched build on all my machines and distros. - Will this CSS-feature make it into 3.12 ? - What should I do to make my version of the feature ready for merge in 3.12 ? - Or is there no chance for this to get merged due to the CSS-stuff and there won't be such a feature in 3.12 ? In the latter case, maybe someone could point me to the current CSS-porting-branch such that I could help out with that code... I really got used to this and would like to see it merged for others but also for my own convenience... ;-) Thanks a lot, Andreas
I don't plan to work on the CSS style schemes any time soon. A property can be added in GtkSourceBuffer to enable/disable font scaling for headings, and add a hidden or visible setting in gedit. What do you think, Paolo?
Review of attachment 245738 [details] [review]: As Paolo said, let's start with the first patches, it would already be nice to get them merged, even if the feature is not available out-of-the-box for users. For the scale property, why not a gfloat type instead of a string? I don't really like the names S, M, L, XL, … In a style scheme, if you see only scale="L", it's not obvious that it means "large". With a gfloat, if you see scale="1.2" or scale="0.8", it's more comprehensible, and also more flexible. ::: gtksourceview/gtksourcestyle-private.h @@ +57,3 @@ guint strikethrough : 1; guint mask : 12; + const gchar *scale; Please move the 'scale' field above the bit fields, just under 'line_background'. It's better to group the bit fields at the end of the struct. ::: gtksourceview/gtksourcestyle.c @@ +504,3 @@ + if (g_strcmp0 (style->scale, "L") == 0) + g_object_set (tag, "scale", PANGO_SCALE_LARGE, NULL); + else if (g_strcmp0 (style->scale, "XL") == 0) Bad indentation for the first 'if'. Also, please use curly braces around blocks. Old code doesn't always have curly braces for one-line blocks, but for new code at least it's better (and it should perhaps be documented in the HACKING file).
Review of attachment 245738 [details] [review]: Another nitpick. ::: data/styles/styles.rng @@ +107,3 @@ <define name="style-elements"> <optional> + <attribute name="scale"/> Please add the scale attribute at the end of 'style-elements'. It's better IMO to have first the common attributes (foreground, background), and at the end the less common attributes. The other option is to sort the attributes in alphabetical order.
(In reply to comment #13) > For the scale property, why not a gfloat type instead of a string? I don't > really like the names S, M, L, XL, … In a style scheme, if you see only > scale="L", it's not obvious that it means "large". With a gfloat, if you see > scale="1.2" or scale="0.8", it's more comprehensible, and also more flexible. The best is probably to do the same as what is available with CSS in GTK+.
Hi Sebastien, first, thanks for reviving this feature... Now to the comments: For scale you can currently use use both: - shortcuts (i.e. S, M, L, ...) that map to pango-equivalents or - a double value (i.e. 1.55). See the last line of the comparison-block: "g_ascii_strtod(style->scale, NULL)" I hope supporting both, a gdouble as well as pango-based shortcut-strings is OK... I'd also suggest not using CSS-like notation, since CSS is more like "font: Arial 12" which we definately do not support here... Since styles-file-schemes have to be changed completely for CSS-transition anyways, I also see no disadvantage where. Is that OK with you ? Field-order: OK Indent: OK (by the way, tab or space-indent ?) rng-Order: OK I'll try to get the patch updated till end of the year... ;-)
I didn't see the g_ascii_strtod(), so it's fine. For the names, why not "large", "x-large", "xx-large", "medium", "small", etc? It would be more understandable in a style scheme .xml file. In CSS, is there a property like font-scale or something similar? The indentation is with tabs, normally each .c and .h file has a modeline at the top, which describes the indentation style (in gsv and gedit, tabs of length 8). Another thing, the reference documentation should be updated to add the scale property.
Yes, normal CSS (for the web) has the font-size property with possible values "xx-small", "x-small", "small", "medium", "large", "x-large", "xx-large". So it's better to use these values. The font-size property also supports "smaller", "larger", a length or a percentage. Like "200%", "0.9em" or "9px". But here with the scale property, "0.9" is ok.
Created attachment 265071 [details] [review] Patch (v2) to add support for scale property in schemes Incorporated the requested changes...
There is a warning that does not make sense to me: (gedit:4517): GtkSourceView-WARNING **: in file /usr/share/gtksourceview-3.0/language-specs/latex.lang: style 'def:heading0' not defined Since "def:heading0" is actually defined inside /usr/share/gtksourceview-3.0/styles/classic.xml. I guess I don't understand the parser yet... Maybe somebody can help me with this. It does not concern the updated patch however, but the subsequent ones only...
Review of attachment 265071 [details] [review]: Thanks for the updated patch. There are still a few minor comments, but don't worry, I've fixed them and pushed the result in the wip/font-scaling branch: https://git.gnome.org/browse/gtksourceview/log/?h=wip/font-scaling WIP stands for work in progress, when the other patches are finished, we can then merge the branch (after rebasing it to have a linear git history). ::: gtksourceview/gtksourcestyle.c @@ +557,3 @@ + if (style->mask & GTK_SOURCE_STYLE_USE_SCALE) { + if (g_ascii_strcasecmp (style->scale, "large") == 0) { The curly braces must be like: if (blah) { } else if (foo) { } @@ +572,3 @@ + g_object_set (tag, "scale", PANGO_SCALE_MEDIUM, NULL); + } else if (g_ascii_strtod(style->scale, NULL) > 0) { + g_object_set (tag, "scale", g_ascii_strtod(style->scale, NULL), NULL); Remember to add a space after a function name.
(In reply to comment #20) > (gedit:4517): GtkSourceView-WARNING **: in file > /usr/share/gtksourceview-3.0/language-specs/latex.lang: style 'def:heading0' > not defined See the file def.lang. It contains common features for language files.
Review of attachment 245739 [details] [review]: This patch looks fine, but before merging it, there should be a mechanism to enable/disable font scaling.
Review of attachment 245741 [details] [review]: Looks almost good. Normally the indentation is 2 spaces, but html.lang has a 4 spaces indentation... This should be fixed later with another commit. ::: data/language-specs/html.lang @@ +42,3 @@ + <style id="h2" _name="Heading type 2" map-to="def:heading2"/> + <style id="h3" _name="Heading type 3" map-to="def:heading3"/> + <style id="h4" _name="Heading type 4" map-to="def:heading4"/> The indentation looks weird here.
(In reply to comment #23) > Review of attachment 245739 [details] [review]: > > This patch looks fine, but before merging it, there should be a mechanism to > enable/disable font scaling. I guess this patch should also include the aditions to def.lang for "def:heading[0..4]" in order to prevent the parsing-errors... I had an idea: How about having a new style called "classic-scaled" that include the new additions ? Would this be a suitable way for "enabling" scaled view; i.e. by choosing a style that includes scaling ?
Review of attachment 245740 [details] [review]: Unfortunately the latex.lang has been modified recently, so there are most probably conflicts. But this seems good. ::: data/language-specs/latex.lang @@ +426,3 @@ <context id="latex"> <include> + <context ref="headings"/> Indentation: 2 spaces.
(In reply to comment #25) > I guess this patch should also include the aditions to def.lang for > "def:heading[0..4]" in order to prevent the parsing-errors... Yes. > I had an idea: > How about having a new style called "classic-scaled" that include the new > additions ? > Would this be a suitable way for "enabling" scaled view; i.e. by choosing a > style that includes scaling ? The problem with that approach is that all styles must have a -scaled counterpart. And this doesn't scale if other features are added later that also require a counterpart style. Like I said above, we can add a property to GtkSourceBuffer or another class, to enable/disable font scaling. If the font scaling is disabled, we just ignore the scale style.
The problem with that is, that we'd need to change gedit, meld and every other consumer of gtksourceview... And it would introduce an additional property setting in each of those... :-( I'm also afraid, propagating this property though all the consumers of gtksourceview would take a few release cycles, not ? The other question with that property is: What should be the default behavior ? With scaling or without scaling ? Therefore, I liked Paolo's idea of additional/custom styles. I'll see for the def.lang and the indent stuff... Thankfully I can rebase ontop of wip/font-scaling now... :-) And regarding the Styles-patch vs Property, I'll act once it's decided... :-)
The -scaled styles would not be shipped by default with GtkSourceView. For the users it is more complicated to install manually a new style than changing the preferences (hidden or visible). The default value in GtkSourceView would be to ignore scaling. An application can easily override this default value though, and is free to provide a setting or not. But in gedit there would be a setting with the same default value.
(In reply to comment #29) > The -scaled styles would not be shipped by default with GtkSourceView. For the > users it is more complicated to install manually a new style than changing the > preferences (hidden or visible). > > The default value in GtkSourceView would be to ignore scaling. An application > can easily override this default value though, and is free to provide a setting > or not. But in gedit there would be a setting with the same default value. Hmmm... I guess now I'll need some help in order to get the right path to pass down the property... So I guess, GtkSourceStyle will gain a property called "enable-scaling". Similar, GtkSourceView will gain a property called "enable-scaling". What's the object-path between those ? Or is it possibly quicker if you made this change, since I did not look at the code anymore since my original patch... :-/
I gave GtkSourceBuffer as an example. With more thoughts, GtkSourceStyleScheme is probably a better place to add a "font-scaling" property. The style scheme can be retrieved from a buffer with gtk_source_buffer_get_style_scheme(). In GtkSourceBuffer it is not a good place I think, because the application can add custom GtkTextTags in the buffer, that are independent of the style schemes.
Created attachment 265181 [details] [review] Fexed a memory leak in the original patch that enables the scale property Could be squashed with the original scale-property patch. Since that's in git already I did not want to change it myself...
Created attachment 265182 [details] [review] Add a (construct-only) property to GtkSourceStyleScheme about the enabling of heading scaling
Created attachment 265183 [details] [review] The necessary additions to def.lang. Separate patch since it should come before the original styles-file-patch.
Created attachment 265184 [details] [review] Patch to use headings from latex langspec. Rework to base of from current latex.lang
Created attachment 265185 [details] [review] Patch to use heading from html langspec. Reworked the weird indentations.
(In reply to comment #31) > I gave GtkSourceBuffer as an example. With more thoughts, GtkSourceStyleScheme > is probably a better place to add a "font-scaling" property. The style scheme > can be retrieved from a buffer with gtk_source_buffer_get_style_scheme(). > > In GtkSourceBuffer it is not a good place I think, because the application can > add custom GtkTextTags in the buffer, that are independent of the style > schemes. As you can see, I provided a new property to GtkSourceStyleScheme for scaling support. However it's construct-only at the moment. If you'd like it as regular property, I wouldn't know how to do that without a major hastle... :-( In case you lost track of the order of patches: - Patch (v2) to add support for scale property in schemes - Fexed a memory leak in the original patch that enables the scale property - Add a (construct-only) property to GtkSourceStyleScheme about the enabling of heading scaling - The necessary additions to def.lang. - Patch to use scale property from styles files - Patch to use headings from latex langspec. Rework to base of from current latex.lang - Patch to use heading from html langspec. Reworked the weird indentations.
Comment on attachment 265181 [details] [review] Fexed a memory leak in the original patch that enables the scale property Pushed to the wip branch. The commit can be squashed when doing the merge.
Comment on attachment 265183 [details] [review] The necessary additions to def.lang. Separate patch since it should come before the original styles-file-patch. Ok, pushed to the wip branch.
Comment on attachment 265184 [details] [review] Patch to use headings from latex langspec. Rework to base of from current latex.lang The diff must be made on top of the wip branch. Here my recent changes are also included.
Review of attachment 265185 [details] [review]: If possible it would be better to fix the issue documented in the commit message. ::: data/language-specs/html.lang @@ +221,3 @@ + </include> + </context> + Try to avoid trailspaces. Git doesn't like trailspaces, if you enable colors you will see it with git diff or git show.
Review of attachment 265182 [details] [review]: As a first step, we will not add the property. Users must install manually a style scheme with headings support. We will see in the future what is the best option for simplifying what the user must do to enable headings. ::: gtksourceview/gtksourcestylescheme.c @@ +83,3 @@ GHashTable *style_cache; GHashTable *named_colors; + gboolean enablescaling; It is better to add booleans in structs as bit fields, like: guint enable_scaling : 1;
Created attachment 265205 [details] [review] Patch (v3) to use heading from html langspec Issue with attributes in h[1-6]-Tags resolved. It would have been easier if I could reference a context from within subpattern->start and not only a style. I think that would make many of the lang-specs way more intuitive. Maybe that would be a worthy new feature... Anyways, it works now...
Created attachment 265206 [details] [review] Patch (v3) to use headings from latex langspec Rebased on current wip/font-scaling. Merges other changes to latex.lang done on that tree...
Review of attachment 265205 [details] [review]: ::: data/language-specs/html.lang @@ +43,3 @@ + <style id="h3" _name="Heading type 3" map-to="def:heading3"/> + <style id="h4" _name="Heading type 4" map-to="def:heading4"/> + <style id="h5" _name="Heading type 5" map-to="def:heading5"/> I would name them "Heading level N" instead. @@ +188,3 @@ + <context id="h1-inside" style-inside="true" style-ref="h1" end-at-line-end="true" end-parent="true"> + <start>></start> + <end><\s*/h1\s*></end> Why repeating the <end>? The 'style' context doesn't repeat the <end>, for example. Is it needed for using style-inside? @@ +271,3 @@ + </include> + </context> + Trail spaces.
Review of attachment 265206 [details] [review]: A few nitpicks. The indentation for .lang files should be 2 spaces. ::: data/language-specs/latex.lang @@ +352,3 @@ + <!-- headings --> + <context id="headings"> Add a blank line after the comment, to be consistent with the other comments. @@ +564,2 @@ <context ref="comment"/> <context ref="verbatim"/> Please ref "headings" after "comment" and "verbatim". For example just before "math" since you added the code before the "math" section.
So the first part seems almost done. Remember also to update the documentation in docs/reference/style-reference.xml.
Created attachment 265294 [details] [review] Patch (v4) to use heading from html langspec Actually, I could not just ommit the inner end-tag, but only the outer end-tag. I did so now... I also found, that I cannot include matching against further html-tags inside, so I removed it. (Otherwise, we would not detect our own end-matching any more) It happens only very rarely that somebody will try to do that. Don't even know if it's allowed...
Created attachment 265295 [details] [review] Patch (v4) to use headings from latex langspec
Created attachment 265296 [details] [review] Patch to add documentation about scale-property to Style-Reference
Created attachment 265297 [details] [review] Patch to add documentation about scale-property to Style-Reference
Comment on attachment 265297 [details] [review] Patch to add documentation about scale-property to Style-Reference Sorry for the double-post, the server gave me an "Internal Error"... P.S. Please mark the original patch as commited if possible...
Comment on attachment 265296 [details] [review] Patch to add documentation about scale-property to Style-Reference Ok, pushed to the wip branch. I've just removed the last line, as the scale style may be used for other things than html headings and latex.
Comment on attachment 245737 [details] Test file html The .tex test file works fine, but the .html test file doesn't work, I don't see the bigger fonts.
(In reply to comment #54) > (From update of attachment 245737 [details]) > The .tex test file works fine, but the .html test file doesn't work, I don't > see the bigger fonts. Hmmm.... ok... Well I can only test this stuff backported to 3.10.1, since I don't have a gtk3.11 around... :-( Was there any change in the lang-file-parser/-handler ?
You can use jhbuild to build all the dependencies from git (glib, gtk+, …). See https://wiki.gnome.org/Apps/Gedit/DevGettingStarted There hasn't been changes to the context engine since a long time. I've checked that the right .lang file is read, and normally it's ok. With test-widget from gsv I see the new heading styles for HTML. I test with the wip/font-scaling-test branch, just pushed: https://git.gnome.org/browse/gtksourceview/log/?h=wip/font-scaling-test
I remember jhbuild... Never really became friends with it though... ;-) I was hoping to work this out in stable and that somebody else could do the forward-porting... But I guess, destiny failed me... So I'm starting setting it up... Guess it will take some time though...
Created attachment 265360 [details] [review] Fix to html.lang problems (squash with html.lang commit) That was obviously a problem on my side. I seem to have accidentally not tested the last change I made, which was reordering the includes at the bottom of html.lang. So tag became higher-priority than headings. Now, heading comes before tags and everything should work. For convinience this patch is based upon wip/font-scaling-test, but it should ultimately be squashed with original html.lang patch I guess... Sorry for this...
Comment on attachment 265360 [details] [review] Fix to html.lang problems (squash with html.lang commit) Thanks, pushed to the wip branch, it works correctly now.
All the commits are merged to the master branch! Thanks for your work and the iterations on the patches. Let's open a new bug report for adding the property to GtkSourceStyleScheme (for example) to enable/disable font scaling more easily. The bug #570939 can be useful for that.
See bug #723310 for the next step.