Re: new module 'system-quote'

[Bruno Haible]

Hi Paul,

> Shouldn't that be system_quote_length (interpreter, string) + 1?

Oops, corrected.

> > # define CMD_FORBIDDEN_CHARS "\n\r"
> 
> That macro is never used -- is that intended?

I'm not sure what to do with strings that contain such characters.
Surely, rejecting them would be better than silent truncation of the
command line. But system_quote* API is not made for returning error
codes...

> Are there restrictions about what can go into the strings?
> For example, can they contain UTF-8 encoding errors?

I believe that the strings need to be correctly encoded in the locale
encoding (which can be GB2312 or Big5, for example), for both
SCI_WINDOWS_CREATEPROCESS and SCI_WINDOWS_CMD. But I can't test it,
because the Windows XP machine I have access to has only 8-bit locale
encodings.

Btw, Windows does not have a locale in UTF-8 encoding.

> shell_quote_argv and system_quote_argv are essentially the same
> function, differing only in that one invokes
> shell_quote_length/shell_quote_copy and the other calls
> system_quote_length/system_quote_copy.  Perhaps they should be
> refactored into calling a single function that is parameterized by
> quoting function.

I agree. The code has now reached a size where the code duplication
makes future changes risky. I'll refactor as you say.

> This refactoring would be easier if we changed the
> signature of system_quote_copy to be this:
> 
>    size_t system_quote_copy (char *p, const char *string);
> 
> so that it returns the length of the string that it creates, and it
> doesn't store anything if !P.  That way, we need just one quoting
> function, which we can pass as an argument to the refactored function
> mentioned above.  E.g., instead of this:
> 
>    size_t length = system_quote_length (interpreter, str) + 2;
>    char *buf = xmalloc (length);
>    char *p = system_quote_copy (buf, interpreter, str);
>    strcpy (p, "\n");
> 
> one would write this:
> 
>    size_t length = system_quote_copy (NULL, interpreter, str) + 2;
>    char *buf = xmalloc (length);
>    char *p = buf + system_quote_copy (buf, interpreter, str);
>    strcpy (p, "\n");

But I don't agree here. I don't find that this set of calling conventions
is easier to remember for the user (except if he's being using wcstombs
all the day).

Bruno