Re: [AD] grabber patch to replace an object

[Eric Botcazou <ebotcazou@xxxxxxxxxx>]

> This allows to replace an object with a new one, useful when you
> maintain an unsorted datafile where object indices are important.

It seems like a useful feature. I have a few remarks:

@@ -197,6 +207,7 @@ static MENU popup_menu[32] =
    { "&Grab",                     grabber,          NULL,       0, NULL  },
    { "&Export",                   exporter,         NULL,       0, NULL  },
    { "&Delete",                   deleter,          NULL,       0, NULL  },
+   { "Replace",                 NULL,             replace_menu, 0, NULL  },
    { "&Rename",                   renamer,          NULL,       0, NULL  },
    { "&Shell Edit",               sheller,          NULL,       0, NULL  },
    { "",                          NULL,             NULL,       0, NULL  },

Did you forget to place an & or are the letters already exhausted?


+   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.


+   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?


@@ -2942,6 +3018,14 @@ static int new_object(void)
 
 
 
+/* handle the replace object command */
+static int replace_object(void)
+{
+   return replacer((int)active_menu->dp);
+}
+
+
+

This doesn't compile on x86-64 because of a warning for a conversion between 
pointers and integers of different sizes. You want

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

instead.

-- 
Eric Botcazou