On Thu, Apr 10, 2014 at 10:35 PM, Dirk Hohndel <[email protected]> wrote:
>
> 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)
>
I agree with you. This system needs to be developed further and polished. I
found it difficult to manually enter the name each and every time. Hence, I
made those changes. I deleted those in patch attached.
>
> > 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 :-)
>
Yes, of course. Made changes regarding this.
>
> +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...
>
Extremely sorry for this. Would not happen again.
>
> +
> +#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
>
>
I made the required changes. The patch is attached.
As you have hinted, there might be more problems in this patch. Please do
mention them so that I can improve both the patch and myself.
Eagerly waiting for your feedbacks.
--
*Venkatesh Shukla *
From b615265205346ea58e0f582c66daafe4f0b5d124 Mon Sep 17 00:00:00 2001
From: Venkatesh Shukla <[email protected]>
Date: Fri, 11 Apr 2014 08:12:06 +0530
Subject: [PATCH] Adds option of saving userid in local files
Userid of Subsurface Webservice can be included in locally saved xml
files and git repository.
For xml files, it is stored in userid tag. For git repo, it is stored
in 00-Subsurface file present in the repo.
Preference dialog and webservice dialog modified to include option
for saving userid locally.
In case of difference in default userid and userid in local file,
some semantics are followed. These can be referred to here:
http://lists.hohndel.org/pipermail/subsurface/2014-April/011422.html
Fixes #473
Signed-off-by: Venkatesh Shukla <[email protected]>
---
dive.h | 6 ++++++
load-git.c | 10 +++++++++-
parse-xml.c | 25 +++++++++++++++++++++++++
qt-ui/preferences.cpp | 11 +++++++++++
qt-ui/preferences.ui | 27 +++++++++++++++++++++++++++
qt-ui/subsurfacewebservices.cpp | 40 ++++++++++++++++++++++++++++++++++++++--
qt-ui/subsurfacewebservices.h | 11 +++++++++++
qt-ui/webservices.ui | 9 ++++++++-
save-git.c | 8 ++++++++
save-xml.c | 5 ++++-
10 files changed, 147 insertions(+), 5 deletions(-)
diff --git a/dive.h b/dive.h
index f62d83d..89497b7 100644
--- a/dive.h
+++ b/dive.h
@@ -887,6 +887,12 @@ extern double strtod_flags(const char *str, const char **ptr, unsigned int flags
#define ascii_strtod(str, ptr) strtod_flags(str, ptr, STRTOD_ASCII)
+extern short save_userid_local;
+extern char* userid;
+
+extern void set_save_userid_local(short value);
+extern void set_userid(char* user_id);
+
#ifdef __cplusplus
}
#endif
diff --git a/load-git.c b/load-git.c
index 4040c16..330f006 100644
--- a/load-git.c
+++ b/load-git.c
@@ -572,6 +572,12 @@ static void parse_trip_notes(char *line, struct membuffer *str, void *_trip)
static void parse_settings_autogroup(char *line, struct membuffer *str, void *_unused)
{ set_autogroup(1); }
+static void parse_settings_userid(char *line, struct membuffer *str, void *_unused)
+{
+ set_save_userid_local(true);
+ set_userid(line);
+}
+
/*
* Our versioning is a joke right now, but this is more of an example of what we
* *can* do some day. And if we do change the version, this warning will show if
@@ -700,7 +706,7 @@ static void trip_parser(char *line, struct membuffer *str, void *_trip)
static struct keyword_action settings_action[] = {
#undef D
#define D(x) { #x, parse_settings_ ## x }
- D(autogroup), D(divecomputerid), D(subsurface), D(version),
+ D(autogroup), D(divecomputerid), D(subsurface), D(userid), D(version),
};
static void settings_parser(char *line, struct membuffer *str, void *_unused)
@@ -1158,6 +1164,8 @@ static int parse_settings_entry(git_repository *repo, const git_tree_entry *entr
git_blob *blob = git_tree_entry_blob(repo, entry);
if (!blob)
return report_error("Unable to read settings file");
+ set_save_userid_local(false);
+ set_userid("");
for_each_line(blob, settings_parser, NULL);
git_blob_free(blob);
return 0;
diff --git a/parse-xml.c b/parse-xml.c
index 483187a..966e567 100644
--- a/parse-xml.c
+++ b/parse-xml.c
@@ -112,6 +112,7 @@ static struct {
} dc;
} cur_settings;
static bool in_settings = false;
+static bool in_userid = false;
static struct tm cur_tm;
static int cur_cylinder_index, cur_ws_index;
static int lastndl, laststoptime, laststopdepth, lastcns, lastpo2, lastindeco;
@@ -858,6 +859,12 @@ static void try_to_fill_sample(struct sample *sample, const char *name, char *bu
nonmatch("sample", name, buf);
}
+void try_to_fill_userid(const char *name, char *buf)
+{
+ if (save_userid_local)
+ set_userid(buf);
+}
+
static const char *country, *city;
static void divinglog_place(char *place, void *_location)
@@ -1367,8 +1374,23 @@ static void divecomputer_end(void)
cur_dc = NULL;
}
+static void userid_start(void)
+{
+ in_userid = true;
+ set_save_userid_local(true); //if the xml contains userid, keep saving it.
+}
+
+static void userid_stop(void)
+{
+ in_userid = false;
+}
+
static void entry(const char *name, char *buf)
{
+ if (in_userid) {
+ try_to_fill_userid(name, buf);
+ return;
+ }
if (in_settings) {
try_to_fill_dc_settings(name, buf);
try_to_match_autogroup(name, buf);
@@ -1515,6 +1537,7 @@ static struct nesting {
{ "weightsystem", ws_start, ws_end },
{ "divecomputer", divecomputer_start, divecomputer_end },
{ "P", sample_start, sample_end },
+ { "userid", userid_start, userid_stop},
/* Import type recognition */
{ "Divinglog", DivingLog_importer },
@@ -1602,6 +1625,8 @@ void parse_xml_buffer(const char *url, const char *buffer, int size,
report_error(translate("gettextFromC", "Failed to parse '%s'"), url);
return;
}
+ set_save_userid_local(false);
+ set_userid("");
reset_all();
dive_start();
doc = test_xslt_transforms(doc, params);
diff --git a/qt-ui/preferences.cpp b/qt-ui/preferences.cpp
index 27570dc..623f327 100644
--- a/qt-ui/preferences.cpp
+++ b/qt-ui/preferences.cpp
@@ -100,6 +100,10 @@ void PreferencesDialog::setUiFromPrefs()
connect(ui.languageFilter, SIGNAL(textChanged(QString)), filterModel, SLOT(setFilterFixedString(QString)));
QSettings s;
+
+ ui.save_uid_local->setChecked(s.value("save_uid_local").toBool());
+ ui.default_uid->setText(s.value("subsurface_webservice_uid").toString().toUpper());
+
s.beginGroup("Language");
ui.languageSystemDefault->setChecked(s.value("UseSystemLanguage", true).toBool());
QAbstractItemModel *m = ui.languageView->model();
@@ -170,6 +174,9 @@ void PreferencesDialog::syncSettings()
{
QSettings s;
+ s.setValue("subsurface_webservice_uid", ui.default_uid->text().toUpper());
+ set_save_userid_local(ui.save_uid_local->checkState());
+
// Graph
s.beginGroup("TecDetails");
@@ -235,6 +242,10 @@ void PreferencesDialog::loadSettings()
QSettings s;
QVariant v;
+
+ ui.save_uid_local->setChecked(s.value("save_uid_local").toBool());
+ ui.default_uid->setText(s.value("subsurface_webservice_uid").toString().toUpper());
+
s.beginGroup("Units");
if (s.value("unit_system").toString() == "metric") {
prefs.unit_system = METRIC;
diff --git a/qt-ui/preferences.ui b/qt-ui/preferences.ui
index 502cc29..485577d 100644
--- a/qt-ui/preferences.ui
+++ b/qt-ui/preferences.ui
@@ -265,6 +265,33 @@
</widget>
</item>
<item>
+ <widget class="QGroupBox" name="groupBox_8">
+ <property name="title">
+ <string>Subsurface Webservice</string>
+ </property>
+ <layout class="QVBoxLayout" name="verticalLayout_7">
+ <item>
+ <widget class="QLabel" name="label_16">
+ <property name="text">
+ <string>Default User ID</string>
+ </property>
+ </widget>
+ </item>
+ <item>
+ <widget class="QLineEdit" name="default_uid">
+ </widget>
+ </item>
+ <item>
+ <widget class="QCheckBox" name="save_uid_local">
+ <property name="text">
+ <string>Save User ID locally?</string>
+ </property>
+ </widget>
+ </item>
+ </layout>
+ </widget>
+ </item>
+ <item>
<spacer name="verticalSpacer_2">
<property name="orientation">
<enum>Qt::Vertical</enum>
diff --git a/qt-ui/subsurfacewebservices.cpp b/qt-ui/subsurfacewebservices.cpp
index a8bce4a..e499228 100644
--- a/qt-ui/subsurfacewebservices.cpp
+++ b/qt-ui/subsurfacewebservices.cpp
@@ -312,12 +312,16 @@ void WebServices::resetState()
SubsurfaceWebServices::SubsurfaceWebServices(QWidget *parent, Qt::WindowFlags f) : WebServices(parent, f)
{
QSettings s;
- ui.userID->setText(s.value("subsurface_webservice_uid").toString().toUpper());
+ if (!save_userid_local || !*userid)
+ ui.userID->setText(s.value("subsurface_webservice_uid").toString().toUpper());
+ else
+ ui.userID->setText(userid);
hidePassword();
hideUpload();
ui.progressBar->setFormat("Enter User ID and click Download");
ui.progressBar->setRange(0, 1);
ui.progressBar->setValue(-1);
+ ui.saveUidLocal->setChecked(save_userid_local);
}
void SubsurfaceWebServices::buttonClicked(QAbstractButton *button)
@@ -339,7 +343,20 @@ void SubsurfaceWebServices::buttonClicked(QAbstractButton *button)
/* store last entered uid in config */
QSettings s;
- s.setValue("subsurface_webservice_uid", ui.userID->text().toUpper());
+ QString qDialogUid = ui.userID->text().toUpper();
+ bool qSaveUid = ui.saveUidLocal->checkState();
+ set_save_userid_local(qSaveUid);
+ if (qSaveUid) {
+ QString qSettingUid = s.value("subsurface_webservice_uid").toString();
+ QString qFileUid = QString::fromStdString(userid);
+ bool s_eq_d = (qSettingUid == qDialogUid);
+ bool d_eq_f = (qDialogUid == qFileUid);
+ if (!d_eq_f || s_eq_d)
+ s.setValue("subsurface_webservice_uid", qDialogUid);
+ set_userid(qDialogUid.toLocal8Bit().data());
+ } else {
+ s.setValue("subsurface_webservice_uid", qDialogUid);
+ }
s.sync();
hide();
close();
@@ -626,6 +643,7 @@ DivelogsDeWebServices::DivelogsDeWebServices(QWidget *parent, Qt::WindowFlags f)
QSettings s;
ui.userID->setText(s.value("divelogde_user").toString());
ui.password->setText(s.value("divelogde_pass").toString());
+ ui.saveUidLocal->hide();
hideUpload();
}
@@ -883,3 +901,21 @@ void DivelogsDeWebServices::buttonClicked(QAbstractButton *button)
break;
}
}
+
+#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);
+}
+
diff --git a/qt-ui/subsurfacewebservices.h b/qt-ui/subsurfacewebservices.h
index 175c991..f3fbb5b 100644
--- a/qt-ui/subsurfacewebservices.h
+++ b/qt-ui/subsurfacewebservices.h
@@ -98,4 +98,15 @@ private:
bool uploadMode;
};
+#ifdef __cplusplus
+extern "C" {
+#endif
+extern short save_userid_local;
+extern char *userid;
+extern void set_save_userid_local(short value);
+extern void set_userid(char *user_id);
+#ifdef __cplusplus
+}
+#endif
+
#endif // SUBSURFACEWEBSERVICES_H
diff --git a/qt-ui/webservices.ui b/qt-ui/webservices.ui
index 295c252..a55b917 100644
--- a/qt-ui/webservices.ui
+++ b/qt-ui/webservices.ui
@@ -45,7 +45,7 @@
</property>
</widget>
</item>
- <item row="4" column="0" colspan="3">
+ <item row="5" column="0" colspan="3">
<widget class="QDialogButtonBox" name="buttonBox">
<property name="orientation">
<enum>Qt::Horizontal</enum>
@@ -79,6 +79,13 @@
</property>
</widget>
</item>
+ <item row="4" column="0" colspan="4">
+ <widget class="QCheckBox" name="saveUidLocal">
+ <property name="text">
+ <string>Save User ID locally?</string>
+ </property>
+ </widget>
+ </item>
<item row="1" column="0">
<widget class="QLabel" name="passLabel">
<property name="text">
diff --git a/save-git.c b/save-git.c
index 634db66..2e8b608 100644
--- a/save-git.c
+++ b/save-git.c
@@ -678,6 +678,13 @@ static struct dir *mktree(struct dir *dir, const char *fmt, ...)
return subdir;
}
+static void save_userid(void *_b)
+{
+ struct membuffer *b = _b;
+ if (save_userid_local)
+ put_format(b, "userid %30s", userid);
+}
+
static void save_one_device(void *_b, const char *model, uint32_t deviceid,
const char *nickname, const char *serial, const char *firmware)
{
@@ -706,6 +713,7 @@ static void save_settings(git_repository *repo, struct dir *tree)
struct membuffer b = { 0 };
put_format(&b, "version %d\n", VERSION);
+ save_userid(&b);
call_for_each_dc(&b, save_one_device);
cond_put_format(autogroup, &b, "autogroup\n");
diff --git a/save-xml.c b/save-xml.c
index 61a9300..0aed920 100644
--- a/save-xml.c
+++ b/save-xml.c
@@ -508,10 +508,13 @@ void save_dives_buffer(struct membuffer *b, const bool select_only)
put_format(b, "<divelog program='subsurface' version='%d'>\n<settings>\n", VERSION);
+ if (save_userid_local)
+ put_format(b, " <userid>%s</userid>\n", userid);
+
/* save the dive computer nicknames, if any */
call_for_each_dc(b, save_one_device);
if (autogroup)
- put_format(b, "<autogroup state='1' />\n");
+ put_format(b, " <autogroup state='1' />\n");
put_format(b, "</settings>\n<dives>\n");
for (trip = dive_trip_list; trip != NULL; trip = trip->next)
--
1.9.0
_______________________________________________
subsurface mailing list
[email protected]
http://lists.hohndel.org/cgi-bin/mailman/listinfo/subsurface