[PATCH weston 3/3] editor: Load a file if specified on command line

[Daniel Stone]

Hi Bryce,

On 20 November 2016 at 22:00, Bryce Harrington <bryce at osg.samsung.com> wrote:
> Add support for basic text file loading, to facilitate more expansive
> testing of its UTF-8 text editing support.

Honestly, I question the value of turning the editor into something
'real': as soon as we're adding a --version argument to something,
we've definitely bought into supporting it to a level that I don't
feel comfortable with. Text editors, much like terminal emulators
(grr) are a never-ending swamp. Some much more concrete issues with
this patch though:

> +/* Join remaining arguments in argv as a space-delimited string.
> + *
> + * Caller is responsible for freeing the returned string.
> + * If there are no remaining arguments, returns a blank string.
> + * Returns NULL on out of memory error.
> +*/
> +char *
> +join_argv(int argc, char *argv[]) {

Warning: no previous declaration (use static). Brace is also on the wrong line.

> +       for (i = 0; i < argc; i++)
> +               buf_size += strlen(argv[i]) + 1;
> +
> +       filename = (char*) malloc(sizeof(char) * (buf_size + 1));
> +       if (filename == NULL)
> +               return NULL;
> +       filename[0] = '\0';
> +
> +       while (argv++, --argc) {
> +               int num = buf_size - offset;
> +               int written = 0;
> +               char space = ' ';
> +
> +               if (argc == 1)
> +                       space = '\0';
> +               written = snprintf(filename+offset, num,
> +                                  "%s%c", *argv, space);
> +               if (num < written)
> +                       break;
> +               offset += written;
> +       }
> +       return filename;
> +}

This is an incredibly novel approach to argument parsing, such that
running 'weston-editor filename with spaces.txt' will attempt to open
'filename with spaces.txt'. I don't know of anything else in UNIX
which does this, nor other text editors. Why not just have the user
quote the arguments, as with everything else?

> +/* Load the contents of a file into a UTF-8 text buffer and return it.
> + *
> + * Caller is responsible for freeing the buffer when done.
> + * On error, returns NULL.
> + */
> +char *
> +read_file(char *filename)
> +{

Another no-previous-declaration warning.

>  int
>  main(int argc, char *argv[])
>  {
>         struct editor editor;
> +       char *text_buffer = NULL;
>         int i;

Warning here: unused variable (introduced with patch 1/3).

I'd be OK with 1/3 if it didn't introduce the warning, but I'd really
rather steer clear of the rest, sorry.

Cheers,
Daniel