DiveShare patch

[Salvo Tomaselli]

Hello

Right I didn't consider windows.

I patched the HTML exporter so that now it can accept NULL and 
not copy the photos.

Also the errors are more "aggregated".

In data domenica 12 ottobre 2014 16:04:05, Dirk Hohndel ha 
scritto:
> On Sun, Oct 12, 2014 at 07:41:41PM +0200, Salvo Tomaselli 
wrote:
> > Greetings,
> > 
> > I made this patch that adds a new export option, to 
diveshare.
> 
> Nice
> 
> > Dives can still be anonymous, but users can click a button 
and
> > go on a page where they have a secret string that they can 
paste
> > so that dives go to their account directly.
> > 
> > Let me know if this can be merged or needs some fixes.
> 
> I'd like to see a few changes... see below
> 
> > Feel free to test it, but make sure that you don't test it 
with
> > any data that you wouldn't want to be public.
> 
> I have NOT tested this, just read the code...
> 
> > diff --git a/qt-ui/diveshareexportdialog.cpp
> > b/qt-ui/diveshareexportdialog.cpp new file mode 100644
> > index 0000000..a44c2ee
> > --- /dev/null
> > +++ b/qt-ui/diveshareexportdialog.cpp
> 
> [...]
> 
> > +void DiveShareExportDialog::finishedSlot(QNetworkReply* 
reply) {
> > +	this->ui->progressBar->setVisible(false);
> > +	if (reply->error() != 0) {
> > +		const char* error;
> > +		switch (reply->error()) {
> > +		case QNetworkReply::ConnectionRefusedError:
> > +			error = "the remote server refused the 
connection (the server is not
> > accepting requests)"; +			break;
> > +		case QNetworkReply::RemoteHostClosedError:
> > +			error ="the remote server closed the 
connection prematurely, before
> > the entire reply was received and processed"; +			
break;
> > +		case QNetworkReply::HostNotFoundError:
> > +			error ="the remote host name was not 
found (invalid hostname)";
> > +			break;
> > +		case QNetworkReply::TimeoutError:
> > +			error ="the connection to the remote 
server timed out";
> > +			break;
> 
> and lots and lots more...
> do we need all these errors? I.e., can they all happen?
> please mark all the texts for translation since they are user 
facing.
> 
> > +		case 
QNetworkReply::TemporaryNetworkFailureError:
> > +			error ="the connection was broken due to 
disconnection from the
> > network, however the system has initiated roaming to another 
access
> > point. The request should be resubmitted and will be 
processed as soon as
> > the connection is re-established.";
> While we have no hard line length limit, this one seems a bit 
excessive...
> 
> > +void DiveShareExportDialog::doUpload() {
> 
> [...]
> 
> > +
> > +	//generate json
> > +	struct membuffer buf = { 0 };
> > +	export_list(&buf, "/tmp",  this->exportSelected, false);
> 
> A path like this will work on Linux and Mac, but not on 
Windows
> 
> This is what jumped out at me after casually reading through 
the code.
> I'll have to spend more time with some of the protocol stuff 
(or get
> Thiago to review that part).
> 
> The reason I didn't pull it and said "let's fix it in future 
commits" is
> that I worry that this is completely untested under Windows 
and I want to
> make sure that we continue to have working "daily" builds on 
that
> platform...
> 
> /D

-- 
Salvo Tomaselli

"Io non mi sento obbligato a credere che lo stesso Dio che ci ha 
dotato di senso, ragione ed intelletto intendesse che noi ne 
facessimo a meno."
                -- Galileo Galilei

                http://ltworf.github.io/ltworf/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: diveshare.patch
Type: text/x-patch
Size: 19130 bytes
Desc: not available
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20141013/3a4d49f3/attachment-0001.bin>