[PATCH] Ticket #473: Add option of saving User ID in xml

[Dirk Hohndel]

Hi Venkatesh,

sorry this took longer than usual - things have been crazy busy for me.
Sadly none of the other developers provided review, either (cough,
cough)...


> Added feature of saving subsurface userid in local files (xml as well
> as the git repo) as per ticket #473
> 
> 
> For git repo, userid is stored in the 00-Subsurface file present in
> the repo. For xml files, it is stored under tag userid.

This seems reasonable.

> Modified preference and webservice dialog to add these options.
> Import, open and save as dialog now show git repo pointer files as
> well. For ex /home/subsurface/linus[dives]

I don't think your patch does what we would want there - adding "*]" as
pattern assumes that this is how those files look in a file browser.
But of course they don't as this is simply magic syntax. This part of
the code (how does a non-expert user figure out saving to a git
repository) is far from done... right now you really need to manually
enter the magic syntax.
(and even if this were correct, I still wouldn't want it mixed in with
this patch as it has nothing to do with saving the userid)

> Complete patch is attached.

Some comments on the patch:

Why is the userid not part of the settings section in the XML file? That
would correspond to the 00-Subsurface file in git :-)

+static void parse_settings_userid(char *line, struct membuffer *str,
void *_unused)
+{
+       char *uid = line;
+       if (uid)
+       {
+               set_save_userid_local(true);
+               set_userid(uid);
+       }
+}

Why do you have that local variable "uid"? Wouldn't it be much easier to
just use "line"?

 
+static void save_userid(void *_b)
+{
+       struct membuffer *b = _b;
+       if (save_userid_local) {
+               put_format(b, "userid %30s", userid);
+               printf("saving userid as save_userid_local is true : %s\n", userid);
+       } else {
+               printf("not saving userid as save_userid_local is false\n");
+       }
+}
+

this looks like some debug output snuck into your final patch...

+
+#ifdef __cplusplus
+extern "C" {
+#endif
+#define MAX_USERID_SIZE 32
+short save_userid_local = false;
+char *userid = NULL;
+void set_save_userid_local(short value)
+{
+       QSettings s;
+       s.setValue("save_uid_local", value);
+       save_userid_local = value;
+}
+
+void set_userid(char *rUserId)
+{
+       userid = (char *) malloc(MAX_USERID_SIZE);
+       if (userid && rUserId)
+               strcpy(userid, rUserId);
+}
+
+#ifdef __cplusplus
+}
+#endif

Why do you have the __cplusplus macro in the .cpp file? You don't need
to do that, only wrap the prototypes in the .h file.

OK, that's what I found so far - maybe others see more issues (hint,
hint) :-)

/D