RE: [AD] grabber patch to replace an object

[Vincent Penquerc'h <Vincent.Penquerch@xxxxxxxxxx>]

Title: RE: [AD] grabber patch to replace an object

> +   { "Replace",                 NULL,            
[...]
> Did you forget to place an & or are the letters already exhausted?

I didn't particuarly want a hotkey, but feel free to add one if you
feel it's appropriate. It's not like it's going to be used a lot,
though it's extremely handy when you need it.

>
> +   name = get_datafile_property(dat, DAT_NAME);
> +   sprintf(buf, "%s?", name);
> +   if (alert("Really delete", buf, NULL, "Yes", "Cancel",
> 'y', 27) != 1)
> +      return D_O_K;
>
> 'name' can be the empty string, you need to special-case it.

It works, because get_datafile_property returns "", not NULL, when
not found (at least last time I checked, but that's months ago).
Or for cosmetics, how about just adding quotes around the name ?
(so "" appears for an empty name) ?

>
> +   if (do_edit("New Object", "Type:", "Name:", type, NULL,
> (type == 0),
> TRUE)) {      if (prop_type_string[0]) {
> +         type = datedit_clean_typename(prop_type_string);
> +         check_valid_name(prop_value_string);
> +
> +         for (i=0; datedit_object_info[i]->type != DAT_END; i++) {
> +            if ((datedit_object_info[i]->type == type) &&
> (datedit_object_info[i]->makenew)) {
> +               v = datedit_object_info[i]->makenew(&size);
> +               break;
> +            }
> +         }
> +
> +         if (!v)
> +            v = makenew_data(&size);
> +      }
> +   }
> +
> +   _unload_datafile_object(dat);
> +   dat->dat = v;
> +   dat->size = size;
> +   dat->type = type;
> +   dat->prop = NULL;
> +   datedit_set_property(dat, DAT_NAME, prop_value_string);
>
> What if do_edit() returns FALSE? (Hint: you get an unexpected
> new file on
> your disk :-) What if 'prop_type_string' is the empty string?

Do you ? Hmm, I didn't notice :) It just deletes the object on my
machine, and then does nothing else. But create a new file... Hmm,
I'll have to check my disk when I get home :)
I didn't check for prop_type_string being empty, though, but that's
evil :) I'll post an updated version later.

>       return replacer((int)(unsigned long)(active_menu->dp));

Ah. That must be in a lot of other places as well then, because
I nicked it from the normal deleter routine IIRC.

--
Vincent Penquerc'h