Re: [gedit-list] i18n fixes for "search and replace" in gedit
- From: Chema Celorio <chema ximian com>
- To: ChiDeok Hwang <hwang mizi co kr>
- Cc: gedit-list lists sourceforge net
- Subject: Re: [gedit-list] i18n fixes for "search and replace" in gedit
- Date: Sun, 11 Mar 2001 03:57:34 -0600
Thank you for the patch, it looks good but I have a couple of comments
* It lacks ChangeLog entries, please add them
> if (settings->font == NULL)
> settings->font = g_strdup (DEFAULT_FONT);
> + if (strchr(settings->font, ',') >= 0)
> + settings->use_fontset = TRUE;
> + else
> + settings->use_fontset = FALSE;
>
This is a "hacky" way to determine if we are using a fontset.
I prefere something like :
if (font->type == GDK_FONT_FONTSET)
...
> + int len_s = mblen(src, MB_CUR_MAX);
> + int len_d;
what is len_s and len_d ? We use longer variables in gedit, even if it
means
that we need to do a little more typing but makes the code easy to
remember what
it did
This :
> + if (len_s == len_d && (len_s == 1 ||
> + ((case_sensitive?strncmp(src, dest, len_s):g_strncasecmp(src, dest, len_s)) == 0)))
and :
> + if ((buffer_in[p1] == search_text[p3]) || (buffer_in[p1] < 0x80 && (buffer_in [p1]|case_sensitive_mask) == (search_text [p3]|case_sensitive_mask)))
are getting a little cryptic. How about if we write a couple of inline
functions
something like :
gedit_search_compare_charcter (const gchar *in_1, const gchar *in_2,
gint case_sensitive_mask)
> if (p3 == search_text_length)
> +char *
> +mb_index(char *s, int pos)
> +{
> + while (pos--) {
> + int i = mblen(s, MB_CUR_MAX);
> + if (i == 0) return s;
> + if (i < 0) s++;
> + else s += i;
> + }
> + return s;
> +}
all the functions in gedit have a prefix, can we find a good name
for this functions ? Again, even if it means more typing but it
has a prefix and they are clear on what they do
gedit_util_string_index
gedit_util_strlen
also please follow the current gedit style.
- we use gchar/gint not chars/int
- the indentation is different than the rest of the code
> +
> +int
> +mb_strlen(const char *s)
> +{
> + int i;
> + int len = 0;
> + while (*s) {
> + i = mblen(s, MB_CUR_MAX);
> + if (i <= 0) i = 1;
> + s += i;
> + len++;
> + }
> + return len;
> +}
> +
same comments here.
Every thing else loooks fine, thank you very much
regards,
Chema
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]