Zsh Mailing List Archive
Messages sorted by:
Reverse Date,
Date,
Thread,
Author
PATCH: Re: Broken completion with UTF-8 description
- X-seq: zsh-workers 22754
- From: Andrey Borzenkov <arvidjaar@xxxxxxxxxx>
- To: zsh-workers@xxxxxxxxxx
- Subject: PATCH: Re: Broken completion with UTF-8 description
- Date: Thu, 21 Sep 2006 21:04:17 +0400
- In-reply-to: <20060917201612.fe9933d5.p.w.stephenson@xxxxxxxxxxxx>
- Mailing-list: contact zsh-workers-help@xxxxxxxxxx; run by ezmlm
- References: <200609171853.57050.arvidjaar@xxxxxxxxxx> <20060917201612.fe9933d5.p.w.stephenson@xxxxxxxxxxxx>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On Sunday 17 September 2006 23:16, Peter Stephenson wrote:
> That's another evening of my life gone.
>
Sorry about that esp. as this did not fixed this particular case due to it
taking different code path.
Patch below seems to correctly compute display length in all cases (where it
is computed in compdesrcibe at all); all my tests using different combination
of ASCII and Russian descriptions and matches passed, including the original
example that started this thread.
How are matches sorted currently? Do we have another function doing collate
sort or completion simply sorts in numerical order? I am not sure if using
strpcmp is the right thing but this is the first I caught.
Half of the code is redundant. It behaves differently when we have to group
matches (with identical descriptions) and when not. Observing that no
grouping case is equal to group of size 1 allows eliminate this duplicated
part. It has one visible effect - now during menu selection in case of
grouping only match itself is highlighted while without grouping the whole
line (match + description) is highlighted. Anyone will miss current behavior?
Code silently depends on another code somewhere inside of completion that I
have never found :/
Please review as I am not sure is usage of ztrdup/zfree is right in this case.
And it became noticeably slow (for Russian strings) on my PIII 733 now ...
- -andrey
Index: Src/subst.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/subst.c,v
retrieving revision 1.61
diff -u -p -r1.61 subst.c
- --- Src/subst.c 20 Sep 2006 09:22:34 -0000 1.61
+++ Src/subst.c 21 Sep 2006 16:51:38 -0000
@@ -581,7 +581,7 @@ strcatsub(char **d, char *pb, char *pe,
typedef int (*CompareFn) _((const void *, const void *));
/**/
- -int
+mod_export int
strpcmp(const void *a, const void *b)
{
#ifdef HAVE_STRCOLL
Index: Src/Zle/computil.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/computil.c,v
retrieving revision 1.95
diff -u -p -r1.95 computil.c
- --- Src/Zle/computil.c 17 Sep 2006 19:23:39 -0000 1.95
+++ Src/Zle/computil.c 21 Sep 2006 16:51:39 -0000
@@ -33,6 +33,13 @@
/* Help for `_describe'. */
+/*
+ * FIXME this should be defined globally. I have to find other places
+ * where it is used
+ * */
+
+#define INTERMATCH_GAP 2
+
typedef struct cdset *Cdset;
typedef struct cdstr *Cdstr;
typedef struct cdrun *Cdrun;
@@ -40,16 +47,18 @@ typedef struct cdrun *Cdrun;
struct cdstate {
int showd; /* != 0 if descriptions should be shown */
char *sep; /* the separator string */
- - int slen; /* its length */
+ int slen; /* its metafied length */
+ int swidth; /* its screen width */
int maxmlen; /* maximum length to allow for the matches */
Cdset sets; /* the sets of matches */
- - int pre; /* longest prefix (before description) */
+ int pre; /* longest prefix length (before description)
*/
+ int premaxw; /* ... and its screen width */
int suf; /* longest suffix (description) */
int maxg; /* size of largest group */
int maxglen; /* columns for matches of largest group */
int groups; /* number of groups */
int descs; /* number of non-group matches with desc */
- - int gpre; /* prefix length for group display */
+ int gprew; /* prefix screen width for group display */
Cdrun runs; /* runs to report to shell code */
};
@@ -59,6 +68,7 @@ struct cdstr {
char *desc; /* the description or NULL */
char *match; /* the match to add */
int len; /* length of str or match */
+ int width; /* ... and its screen width */
Cdstr other; /* next string with the same description */
int kind; /* 0: not in a group, 1: the first, 2: other
*/
Cdset set; /* the set this string is in */
@@ -123,7 +133,7 @@ cd_group(int maxg)
{
Cdset set1, set2;
Cdstr str1, str2, *strp;
- - int num, len;
+ int num, width;
cd_state.groups = cd_state.descs = cd_state.maxglen = 0;
cd_state.maxg = 0;
@@ -140,20 +150,20 @@ cd_group(int maxg)
continue;
num = 1;
- - len = str1->len + cd_state.slen;
- - if (len > cd_state.maxglen)
- - cd_state.maxglen = len;
+ width = str1->width + cd_state.swidth;
+ if (width > cd_state.maxglen)
+ cd_state.maxglen = width;
strp = &(str1->other);
for (set2 = set1; set2; set2 = set2->next) {
for (str2 = (set2 == set1 ? str1->next : set2->strs);
str2; str2 = str2->next)
if (str2->desc && !strcmp(str1->desc, str2->desc)) {
- - len += 2 + str2->len;
- - if (len > cd_state.maxmlen || num == maxg)
+ width += INTERMATCH_GAP + str2->width;
+ if (width > cd_state.maxmlen || num == maxg)
break;
- - if (len > cd_state.maxglen)
- - cd_state.maxglen = len;
+ if (width > cd_state.maxglen)
+ cd_state.maxglen = width;
str1->kind = 1;
str2->kind = 2;
num++;
@@ -194,6 +204,8 @@ cd_calc()
set->count++;
if ((l = strlen(str->str)) > cd_state.pre)
cd_state.pre = l;
+ if ((l = MB_METASTRWIDTH(str->str)) > cd_state.premaxw)
+ cd_state.premaxw = l;
if (str->desc) {
set->desc++;
if ((l = strlen(str->desc)) > cd_state.suf)
@@ -206,7 +218,18 @@ cd_calc()
static int
cd_sort(const void *a, const void *b)
{
- - return strcmp((*((Cdstr *) a))->str, (*((Cdstr *) b))->str);
+ char *as = ztrdup((*((Cdstr *) a))->str);
+ int aslen = strlen(as);
+ char *bs = ztrdup((*((Cdstr *) b))->str);
+ int bslen = strlen(bs);
+ int ret;
+
+ unmetafy(as, &ret);
+ unmetafy(bs, &ret);
+ ret = strpcmp(&as, &bs);
+ zfree(as, aslen);
+ zfree(bs, bslen);
+ return ret;
}
static int
@@ -234,8 +257,8 @@ cd_prep()
for (str = set->strs; str; str = str->next) {
if (str->kind != 1) {
if (!str->kind && str->desc) {
- - if (str->len > wids[0])
- - wids[0] = str->len;
+ if (str->width > wids[0])
+ wids[0] = str->width;
str->other = NULL;
*strp++ = str;
}
@@ -248,23 +271,24 @@ cd_prep()
for (; gp; gp = gn) {
gn = gp->other;
gp->other = NULL;
- - for (gpp = &gs; *gpp && (*gpp)->len > gp->len;
+ for (gpp = &gs; *gpp && (*gpp)->width > gp->width;
gpp = &((*gpp)->other));
gp->other = *gpp;
*gpp = gp;
}
for (gp = gs, i = 0; gp; gp = gp->other, i++)
- - if (gp->len > wids[i])
- - wids[i] = gp->len;
+ if (gp->width > wids[i])
+ wids[i] = gp->width;
*strp++ = gs;
}
- - cd_state.gpre = 0;
- - for (i = 0; i < cd_state.maxg; i++)
- - cd_state.gpre += wids[i] + 2;
+ cd_state.gprew = 0;
+ for (i = 0; i < cd_state.maxg; i++) {
+ cd_state.gprew += wids[i] + INTERMATCH_GAP;
+ }
- - if (cd_state.gpre > cd_state.maxmlen && cd_state.maxglen > 1)
+ if (cd_state.gprew > cd_state.maxmlen && cd_state.maxglen > 1)
return 1;
qsort(grps, lines, sizeof(Cdstr), cd_sort);
@@ -443,12 +467,13 @@ cd_init(char *nam, char *hide, char *mle
}
setp = &(cd_state.sets);
cd_state.sep = ztrdup(sep);
- - cd_state.slen = ztrlen(sep);
+ cd_state.slen = strlen(sep);
+ cd_state.swidth = MB_METASTRWIDTH(sep);
cd_state.sets = NULL;
cd_state.showd = disp;
cd_state.maxg = cd_state.groups = cd_state.descs = 0;
cd_state.maxmlen = atoi(mlen);
- - itmp = columns - cd_state.slen - 4;
+ itmp = columns - cd_state.swidth - 4;
if (cd_state.maxmlen > itmp)
cd_state.maxmlen = itmp;
if (cd_state.maxmlen < 4)
@@ -489,6 +514,7 @@ cd_init(char *nam, char *hide, char *mle
*tmp = '\0';
str->str = str->match = ztrdup(rembslash(*ap));
str->len = strlen(str->str);
+ str->width = MB_METASTRWIDTH(str->str);
}
if (str)
str->next = NULL;
@@ -600,38 +626,57 @@ cd_get(char **params)
case CRT_DESC:
{
+ /*
+ * The buffer size:
+ * max prefix length (cd_state.pre) +
+ * max padding (cd_state.premaxw generously :) +
+ * separator length (cd_state.slen) +
+ * inter matches gap (INTERMATCH_GAP) +
+ * max description length (cd_state.suf) +
+ * trailing \0
+ */
VARARR(char, buf,
- - cd_state.pre + cd_state.suf + cd_state.slen + 3);
- - char *sufp = NULL;
- -
- - memcpy(buf + cd_state.pre + 2, cd_state.sep, cd_state.slen);
- - buf[cd_state.pre] = buf[cd_state.pre + 1] = ' ';
- - sufp = buf + cd_state.pre + cd_state.slen + 2;
- -
+ cd_state.pre + cd_state.suf +
+ cd_state.premaxw + cd_state.slen + 3);
mats = mp = (char **) zalloc((run->count + 1) * sizeof(char
*));
dpys = dp = (char **) zalloc((run->count + 1) * sizeof(char
*));
for (str = run->strs; str; str = str->run) {
+ char *p = buf, *pp, *d;
+ int l, remw;
+
*mp++ = ztrdup(str->match);
- - memset(buf, ' ', cd_state.pre);
- - memcpy(buf, str->str, str->len);
- - strcpy(sufp, str->desc);
- - if (MB_METASTRWIDTH(buf) >= columns - 1) {
- - char *termptr = buf;
+ strcpy(p, str->str);
+ p += str->len;
+ memset(p, ' ', (l = (cd_state.premaxw - str->width +
INTERMATCH_GAP)));
+ p += l;
+ strcpy(p, cd_state.sep);
+ p += cd_state.slen;
+
+ /*
+ * copy a character at once until no more screen width
+ * is available. Leave 1 character at the end of screen
+ * as safety margin
+ */
+ remw = columns - cd_state.premaxw - cd_state.swidth - 3;
+ pp = p;
+ d = str->desc;
+ while (*d) {
int w;
- - MB_METACHARINIT();
- - for (w = columns - 1; *termptr && w > 0; ) {
- - convchar_t cchar;
- - int cw;
- - termptr += MB_METACHARLENCONV(termptr, &cchar);
- - cw = WCWIDTH(cchar);
- - if (cw >= 0)
- - w -= cw;
- - else
- - w--;
+
+ l = MB_METACHARLEN(d);
+ memcpy(pp, d, l);
+ pp[l] = '\0';
+ w = MB_METASTRWIDTH(p);
+ if (w > remw) {
+ *pp = '\0';
+ break;
}
- - *termptr = '\0';
+
+ pp += l;
+ d += l;
}
+
*dp++ = ztrdup(buf);
}
*mp = *dp = NULL;
@@ -684,10 +729,10 @@ cd_get(char **params)
default: /* This silences the "might be used uninitialized" warnings */
case CRT_EXPL:
{
- - int dlen = columns - cd_state.gpre - cd_state.slen;
- - VARARR(char, dbuf, dlen + cd_state.slen);
- - char buf[20];
- - int i = run->count;
+ /* add columns as safety margin */
+ VARARR(char, dbuf, cd_state.suf + cd_state.slen + columns);
+ char buf[20], *p, *pp, *d;
+ int i = run->count, remw, w, l;
sprintf(buf, "-E%d", i);
@@ -699,13 +744,29 @@ cd_get(char **params)
*dp++ = ztrdup("");
continue;
}
- - memset(dbuf + cd_state.slen, ' ', dlen - 1);
- - dbuf[dlen + cd_state.slen - 1] = '\0';
+
strcpy(dbuf, cd_state.sep);
- - memcpy(dbuf + cd_state.slen,
- - str->desc,
- - ((int)strlen(str->desc) >= dlen ? dlen - 1 :
- - (int)strlen(str->desc)));
+ remw = columns - cd_state.gprew - cd_state.swidth - INTERMATCH_GAP;
+ p = pp = dbuf + cd_state.slen;
+ d = str->desc;
+ while (*d) {
+ l = MB_METACHARLEN(d);
+ memcpy(pp, d, l);
+ pp[l] = '\0';
+ w = MB_METASTRWIDTH(p);
+ if (w > remw) {
+ *pp = '\0';
+ break;
+ }
+
+ pp += l;
+ d += l;
+ }
+
+ while (w++ < remw)
+ *pp++ = ' ';
+ *pp = '\0';
+
*dp++ = ztrdup(dbuf);
}
mats[0] = *dp = NULL;
- -andrey
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (GNU/Linux)
iD8DBQFFEsYXR6LMutpd94wRAqqWAJ9++5aNYVpYL2sbkkWi2jo/Ty/EIwCgy0SP
fNR74jf1tEXziM2Z8L1CJak=
=Df6S
-----END PGP SIGNATURE-----
Messages sorted by:
Reverse Date,
Date,
Thread,
Author