On Sun, 8 Apr 2012 22:02:35 +0100, Thomas Adam <tho...@fvwm.org> wrote:
> On Wed, Apr 04, 2012 at 05:18:01PM +0200, Anton Khirnov wrote:
> > ---
> > NEWS | 3 ++
> > modules/FvwmPager/FvwmPager.1.in | 4 ++
> > modules/FvwmPager/FvwmPager.c | 5 +++
> > modules/FvwmPager/x_pager.c | 62
> > ++++++++++++++++++++++++++++++++++++-
> > 4 files changed, 72 insertions(+), 2 deletions(-)
> >
> > diff --git a/NEWS b/NEWS
> > index 44248bd..c69d694 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -6,6 +6,9 @@ Changes in CVS HEAD (not yet released)
> >
> > * New features:
> >
> > + - FvwmPager now accepts a "WrapLabels" option for wrapping window
> > + labels.
> > +
>
> I really don't think having an option for this is necessary. It should just
> be the default. Please re-roll this patch assuming that, since I don't have
> the time to mangle it myself. But see comments below to include other
> things.
>
> > +/* draw the window label with simple greedy wrapping */
> > +static void label_window_wrap(PagerWindow *t)
> > +{
> > + char *cur = t->window_label, *next = cur;
> > + char *end = cur + strlen(cur);
>
> Can you not do this, and instead make declaration and assignment different
> things? Whilst this is still valid for block-wise code for C89, it doesn't
> match the albeit loose coding style we're meant to have, and whilst I'm
> entirely guilty of it myself, there's already surrounding code which does
> the "preferred" thing, so I'd rather see:
>
> char *cur, *next, *end;
> ...
> cur = t->whatever;
>
>
> > + int space_width = FlocaleTextWidth(FwindowFont, " ", 1);
> > + int cur_width = 0;
> > +
> > + while (*cur) {
> > + while (*next) {
> > + int width;
> > + char *p = strchr(next, ' ');
> > +
> > + if (!p)
> > + p = end;
> > +
> > + width = FlocaleTextWidth(FwindowFont, next, p - next );
> > + if (width > t->pager_view_width - cur_width - space_width - 4)
>
> 4 as a magic number? Use a #define somewhere, please.
>
All those fixed.
> > + break;
> > + cur_width += width + space_width;
> > + next = *p ? p + 1 : p;
> > + }
> > +
> > + if (cur == next) {
> > + /* word too large for window */
> > + while (*next) {
> > + int len = FlocaleStringNumberOfBytes(FwindowFont, next);
> > + int width = FlocaleTextWidth(FwindowFont, next, len);
> > +
> > + if (width > t->pager_view_width - cur_width - 4 && cur != next)
> > + break;
> > +
> > + next += len;
> > + cur_width += width;
> > + }
> > + }
>
> So if I'm reading the above correctly, you first scan the string looking for
> spaces and use that as the tokenising for making the text flow, otherwise if
> that fails, you then check if it's too large a string without a space and
> wrap it as best you can? I think it would be better to just reflow the
> entire string, and not worry about the spaces, so that if you had this as a
> window title:
>
> "This is my window"
>
> And a width in FvwmPager of 10, then I would rather see it wrapped as:
>
> "This is my
> window"
You mean disregard word boundaries and just fill each line completely?
I'd rather not do that, word wrapping looks better to me (at least in my
setup, compare [1] and [2]). I can of course make it configurable if you
want, it's just a matter of adding one condition to the inner while().
[1] http://i.imgur.com/EfZ9d.jpg
[2] http://i.imgur.com/u6GuS.jpg
--
Anton Khirnov>From d6e7834fb4d4dfe9f3b13725d72f522259ea6abc Mon Sep 17 00:00:00 2001
From: Anton Khirnov <an...@khirnov.net>
Date: Mon, 9 Apr 2012 08:00:54 +0200
Subject: [PATCH] FvwmPager: wrap window labels.
---
modules/FvwmPager/x_pager.c | 65 +++++++++++++++++++++++++++++++++++++++++--
1 files changed, 62 insertions(+), 3 deletions(-)
diff --git a/modules/FvwmPager/x_pager.c b/modules/FvwmPager/x_pager.c
index 68a6943..a0c3a6f 100644
--- a/modules/FvwmPager/x_pager.c
+++ b/modules/FvwmPager/x_pager.c
@@ -119,6 +119,7 @@ static char l_g_bits[] = {0x08, 0x02};
#define s_g_height 4
static char s_g_bits[] = {0x01, 0x02, 0x04, 0x08};
+#define label_border g_width
Window icon_win; /* icon window */
@@ -2739,6 +2740,64 @@ void BorderIconWindow(PagerWindow *t)
t, t->IconView, t->icon_view_width, t->icon_view_height);
}
+/* draw the window label with simple greedy wrapping */
+static void label_window_wrap(PagerWindow *t)
+{
+ char *cur, *next, *end;
+ int space_width, cur_width;
+
+ space_width = FlocaleTextWidth(FwindowFont, " ", 1);
+ cur_width = 0;
+
+ cur = next = t->window_label;
+ end = cur + strlen(cur);
+
+ while (*cur) {
+ while (*next) {
+ int width;
+ char *p;
+
+ if (!(p = strchr(next, ' ')))
+ p = end;
+
+ width = FlocaleTextWidth(FwindowFont, next, p - next );
+ if (width > t->pager_view_width - cur_width - space_width - 2*label_border)
+ break;
+ cur_width += width + space_width;
+ next = *p ? p + 1 : p;
+ }
+
+ if (cur == next) {
+ /* word too large for window */
+ while (*next) {
+ int len, width;
+
+ len = FlocaleStringNumberOfBytes(FwindowFont, next);
+ width = FlocaleTextWidth(FwindowFont, next, len);
+
+ if (width > t->pager_view_width - cur_width - 2*label_border && cur != next)
+ break;
+
+ next += len;
+ cur_width += width;
+ }
+ }
+
+ FwinString->str = safemalloc(next - cur + 1);
+ strncpy(FwinString->str, cur, next - cur);
+ FwinString->str[next - cur] = 0;
+
+ FlocaleDrawString(dpy, FwindowFont, FwinString, 0);
+
+ free(FwinString->str);
+ FwinString->str = NULL;
+
+ FwinString->y += FwindowFont->height;
+ cur = next;
+ cur_width = 0;
+ }
+}
+
static void do_label_window(PagerWindow *t, Window w)
{
int cs;
@@ -2776,7 +2835,6 @@ static void do_label_window(PagerWindow *t, Window w)
{
if (FftSupport && FwindowFont != NULL && FwindowFont->fftf.fftfont != NULL)
XClearWindow(dpy, w);
- FwinString->str = t->window_label;
FwinString->win = w;
FwinString->gc = Scr.NormalGC;
FwinString->flags.has_colorset = False;
@@ -2785,9 +2843,10 @@ static void do_label_window(PagerWindow *t, Window w)
FwinString->colorset = &Colorset[cs];
FwinString->flags.has_colorset = True;
}
- FwinString->x = 2;
+ FwinString->x = label_border;
FwinString->y = FwindowFont->ascent+2;
- FlocaleDrawString(dpy, FwindowFont, FwinString, 0);
+
+ label_window_wrap(t);
}
}
--
1.7.9.1
