Re: [gnugo-devel] sgf cleaning continued

[Gunnar Farneback]

Paul wrote:
> changes:
>   - fixed a bug in gunnar_3_9.3 (see sgftreeStartVariantFirst()),
>     changed some comments ;)
>   - gameinfo_play_sgftree_rot() now updates lastnode field. some modes
>     (at least ascii and load and analyze) work better now: when gnu go
>     is launched with -L option they add variations at proper places.
>   - changed parameter of gameinfo_play_sgftree_rot() from SGFNode * to
>     SGFTree * (tree root is always passed anyway). caused some changes
>     in play_gtp.c and debugboard/main.c.
>   - two new functions: sgftreeBack() and sgftreeForward() for
>     navigating through trees with lastnode field.
>   - play_ascii.c mode now uses sgftree.. functions instead of sgf..
>     ones (as in Gunnar's latest patch). so, no more curnode variable,
>     uses lastnode field instead.
>   - a couple of other minor changes in play_ascii.c (killed
>     ascii2pos(), changed parameters of init_sgf()).

This patch mostly looks good except that it doesn't apply. Apparently
tabs in the context has been converted to spaces in the patch causing
patch to detect conflicting changes. This can be worked around with
the -l option to patch (ignores whitespace) but then the patch still
turns out to be malformed.

Some general advice for submitting patches by mail:
* Make sure that the patch comes through unchanged. The best practice
  is to redirect the output from cvs diff to a file and then import
  the file into your mail program. Something you should not do is to
  cut and paste the patch from a terminal window. That will invariably
  introduce errors. Cut and paste from an editor may or may not work
  correctly but at least has better chance to do.
* Never edit the patch unless you know exactly what you do. Instead
  make the changes in the involved files themselves and make a new
  patch. The exception is lines like "? patterns/handipat.c" appearing
  before the first "Index:" line. Those are always safe to remove and
  it's encouraged practice to do so.
* Make sure your mailer doesn't change any formatting on it's own.

>  static void
>  ascii_goto(Gameinfo *gameinfo, char *line)
>  {
> -  int movenumber = 0;
> +
>    if (!line)
>      return;
>    if (!strncmp(line, "last", 4))
> -    movenumber = 9999;
> +    movenumber = "9999";
>    else {
>      if (!strncmp(line, "first", 4))
> -      movenumber = 1;
> -    else
> -      sscanf(line, "%i", &movenumber);
> +      movenumber = "1";
>    }
> -  printf("goto %i\n", movenumber);
> -  curnode = sgftree.root;
> -  gameinfo->to_move = gnugo_play_sgftree(curnode, &movenumber, &curnode);
> -  return;
[...]
> +  printf("goto %s\n", movenumber);
> +  gameinfo_play_sgftree(gameinfo, &sgftree, movenumber);
>  }

This looks strange. The movenumber variable is no longer declared at
all and it looks like it's only possible to go to the first and last
moves, not to an arbitrary move number.

/Gunnar