[PATCH] Choose syntax

[Roland Illig]

Leonard den Ottolander wrote:
> Hi Roland,
> 
> On Mon, 2005-07-11 at 08:33, Roland Illig wrote:
> 
>>>+exec_syntax_dialog (const char **names) {
>>
>>This function must know the size of the ''names'' buffer. I suggest a 
>>second parameter: size_t names_size.
> 
> 
> The problem is that we don't know the length of names before the
> function finishes. I work around this by always supplying a sufficiently
> large list of MAX_SYNTAX_FILES items. Making the length configurable
> seems not to make much sense. The best approach of course would be to
> dynamically grow the list, but I haven't yet succeeded in doing that.
> IIANM it requires the passing of a char** by reference, ie a char***.
> The code I came up with so far kept segfaulting ;-) .

I think I din't say clear enough what I meant (my fault): 
exec_syntax_dialog should not use the constant MAX_SYNTAX_FILES, but 
some parameter names_size. That makes the relation between them tight 
closer. Similar examples using this style are: g_strlcpy(dest, src, 
destsize), g_snprintf(dest, destsize, ...) and some others.

>>>+	    ! strcmp (names[i], option_syntax_type))
>>
>>don't use !strcmp(...). This function does not return a boolean, but a 
>>"comparison result", which should be checked using the relational 
>>operators (==, <=, >=, <, >).
> 
> I know. But as I am only interested in equality of strings ('==' returns
> 0, !'==' 1) strcmp suffices for what I want to do.

I meant: _use_ strcmp, but don't use boolean operators with it. That is:

	if (strcmp (names[i], option_syntax_type) == 0) ...

Don't ever use boolean operators if you don't intend them to _mean_ 
logic manipulation.

In HEAD, I placed a nice macro into src/util.h:

/* usage: str_cmp ("foo", !=, "bar") */
#define str_cmp(a,rel,b) (strcmp ((a), (b)) rel 0)

To me it looks quite good, although it does not follow the usual C syntax.

>>>@@ -272,6 +272,13 @@ menu_options (void)
>>> {
>>>     edit_options_dialog ();
>>> }
>>>+
>>>+static void
>>>+menu_syntax (void)
>>>+{
>>>+    syntax_dialog ();
>>>+}
>>>+
>>
>>Why this extra function?
> 
> 
> Again following common practice of wrapping menu functions before
> calling the actual dialog function.

cvs diff -r1.19 -r1.20 edit/editmenu.c

It's like I expected it. That was not "common practice" but superfluous 
from the beginning, hidden inside a whole bunch of functions where the 
wrapper was actually necessary. But not in that one case. So both 
unnecessary wrappers (menu_options and menu_syntax) should be removed.

Roland