[PATCH] Enable Ctrl+ / Ctrl- for font increase/decrease

Submitted by Luca Errani

Assigned to Matthew Brush @matt

Link to original bug (#16376)

Description

Created attachment 9379 Patch file including the ability to Ctrl+/Ctrl- to increase/decrease font size

Hi everyone,

I use Mousepad everyday as my main text-editor but I found that it lacks the "Zoom in/Zoom out" feature.

I added this patch in order to make it work, I'm sure as hell there are way more elegant ways to achieve the same result, but this is my contribution.

I read on the mailing list that submitting a bug here on Bugzilla is the right way to show you a patch for a XFCE's project. I'm pretty new to working in repositories this big, so I hope I got it right.

Of course, I'm widely open to critics and corrections.

Patch 9379, "Patch file including the ability to Ctrl+/Ctrl- to increase/decrease font size":
0001-Ctrl-Ctrl-increase-font-s-size-by-2.patch

Version: 0.4.2

🛠 Luca Errani submitted a patch:

EDIT: The new patch fixes a small copy/paste error I missed when preparing the previous one

Patch 9382, "[FIX] This patch fixes the previous one where a silly copy/paste error failed compile":
0001-Ctrl-Ctrl-can-increase-decrease-font-size.patch

💬 Matthew Brush @matt said:

I haven't thoroughly reviewed or tested this yet, but a glance, it looks like it could be made more robust by using the Pango API, like to parse the font description: https://developer.gnome.org/pango/stable/pango-Fonts.html#pango-font-description-from-string

Also it might be nicer (though a bit more work) to add a "zoom-level" property to the MousepadView rather than embedding the logic in the MousepadWindow code, and then having the action handler just update the new property.

To be honest I'm quite surprised GtkSourceView has no zoom-in/zoom-out functionality itself. It seems a bit odd changing the font preference to achieve zooming.

💬 Luca Errani said:

You're completely right, I could implement it using Pango API, I don't find very suitable the "atoi" implementation too.

Speaking of "how to achieve" the zoom effect, I just copied the way that the IntelliJ-suite follow so, if you think we should find another way, let's close this bug and go on, otherwise I could upload a new patch using pango library which will increase the font size in the preferences and see how to deal with it!

Thank you very much for your commentm I'll work on that!

🛠 Luca Errani submitted a patch:

I modified my patch on Matthew's suggestions, now it uses Pango API; all the logic is still in mousepad-window.c though

Patch 9387, "[PATCH] Use of Pango API to retrieve and set font size":
diff

💬 Theo Linkspfeifer said:

Thunar, Geany and others have an option to reset back to normal size (Ctrl+0). Maybe this should be added also.

💬 Luca Errani said:

Hi Theo! This shouldn't be hard to implement, so I'll do it. Is there a default value stored in mousepad's settings? I only saw the "USE_DEFAULT_FONT" macro but I don't think we should reset the selected font to default; do you think we should set a "default font size" value somewhere?

💬 Theo Linkspfeifer said:

It is defined here: https://git.xfce.org/apps/mousepad/tree/mousepad/mousepad-view.c#n44

🛠 Luca Errani submitted a patch:

Hi everyone! I followed your suggestions, now MousepadWindow simply updates the corresponding MousepadView's zoom_level property, so that Ctrl++ and Ctrl+- set its value to the previous +2 or the previous -2.

As Theo suggested, I also added the ability to reset to default font size by pressing Ctrl+0.

Let me know if it's better than before and if we have to work more on it!

Patch 9393, "[PATCH] Moved zoom_level in MousepadView as a property, added Ctrl+0 to reset font size":
diffs

💬 Matthew Brush @matt said:

Nice work!

The only thing I don't like is that the zoom setting affects the font setting, though I'm not sure there's a better way.

💬 Luca Errani said:

Hi Matthew!

Thank you! I understand that manually interacting with the font setting could become messy if not treated the right way, but I saw that even Gedit uses the TextSize plugin to achieve the same result, and it increases/decreases the font size too!

Anyway, let me know if any other modification is needed!

💬 Matthew Brush @matt said:

I tested this out a bit, a few comments:

If you build with -Wignored-qualifiers (default on with my compiler/environment), there are warnings about returning const gint instead of gint, since it's already returned by value, I suppose.

Also with -Wreturn-type (again default with my compiler/environment), it warns about a g_return_if_fail use in mousepad_view_set_default_zoom_level() since it returns a value. It should use g_return_val_if_fail instead with a sensible 2nd argument to be returned in case of failure.

As for using it, the only glitch I noticed is that when you zoom in and out and reset the zoom, it doesn't seem to update the GtkFontButton font size in the preferences dialog. Here it ends up showing 0 for the font size that is part of the GtkFontButton label.

💬 Matthew Brush @matt said:

(In reply to Matthew Brush from comment 11)

I tested this out a bit, a few comments:

If you build with -Wignored-qualifiers (default on with my compiler/environment), there are warnings about returning const gint instead of gint, since it's already returned by value, I suppose.

Also with -Wreturn-type (again default with my compiler/environment), it warns about a g_return_if_fail use in mousepad_view_set_default_zoom_level() since it returns a value. It should use g_return_val_if_fail instead with a sensible 2nd argument to be returned in case of failure.

As for using it, the only glitch I noticed is that when you zoom in and out and reset the zoom, it doesn't seem to update the GtkFontButton font size in the preferences dialog. Here it ends up showing 0 for the font size that is part of the GtkFontButton label.

Also one subjective thing which can be ignored, but IMO there should be a menu separator between the "Select Font" and the new "Zoom In/Out/Default" menu items in the View menu in mousepad-window-ui.xml. Even though they are technically related, I don't think they should be related in the UI.

🛠 Luca Errani submitted a patch:

Hi Matthew!

I fixed the warnings you suggested, added a separator between "Select font" and the "Zoom in/out/default" block and fixed the missing update in the settings panel.

Though I have to say that it behaves a little bit strangely, everytime I try to zoom in/out, the mousepad_view_update_font gets called twice: one from the propery binding, one from the explicit call in mousepad_view_set_zoom_level, but removing the latter will result in no update at all. Do you know why? I've tried to fix it but I didn't manage to find a solution.

Patch 9396, "[PATCH] Updates preferences on zoom in/out, View menu separator":
dif

🛠 Luca Errani submitted a patch:

I found the problem! Basically I was calling "mousepad_view_set_zoom_level", then setting to false the USE_DEFAULT_FONT setting and set the ZOOM_LEVEL property to its new value. All these three functions were calling "mousepad_view_update_font", which resulted in a silly overhead. Now it doesn't call the "mousepad_view_set_zoom_level" and sets to fasle the USE_DEFAULT_FONT only if needed.

The instruction MOUSEPAD_SETTING_SET_BOOLEAN(ZOOM_LEVEL, zoom_level) will do the trick and update the view

Patch 9397, "[PATCH] fix for multiple "update font" calls when zooming":
di

💬 Luca Errani said:

Hi! Can I have an update about this thread? Does it need to be reviewed or edited? What can I do to make it better?

💬 Theo Linkspfeifer said:

I applied your patch and did a quick test. It seems to work.

However, you should change the menu entry labels to Title Case and maybe move the entries to the bottom of the View menu (see Geany).

💬 Theo Linkspfeifer said:

Furthermore, I noticed that both Geany and Thunar use "Normal Size" as label. This could be adapted also.

🛠 Luca Errani submitted a patch:

Hi! I added a patch based on Theo's suggestion! Hope it's good!

Patch 9408, "[PATCH] Label refactor":
diff

mentioned in issue #54 (closed)

I tried to update this patch mostly mechanically and fixing whitespace issues, can people please review/test?

It seemed the first time I tried it, it made the fonts huge, but afterwards it seemed to work as expected. Can anyone confirm?

text-zoom.patch