Open / Save to cloud on Windows 10

[Dirk Hohndel]

Hi Jérémie

On Thu, Feb 23, 2017 at 07:31:20PM +0700, Jérémie Guichard wrote:
> Hi guys,
> 
> Pull request sent (https://github.com/Subsurface-divelog/subsurface/pull/222
> )
> Sorry for the late reply, these days were quite busy with a bit of coding
> but mainly with my Instructor Training Course :)

It didn't seem late at all. Usually it takes people a few days to
implement things that go beyond a "quick fix".

> Based on your recommendations I've added a subsurface_stat portability
> function. There I used the corresponding widechar implementation wstat()
> for windows (after utf8 to utf16 conversion) and simply called the normal
> stat() otherwise. It seems to work nicely.

And sounds very reasonable.

> From what I understand when reading msdn, stat() is actually expecting a
> path encoded with the codepoint that is defined in system settings so it
> can be many things but rarely utf8... Win32 only apps do not face issues
> since they would read this from user input or other win32 api that are
> already returning paths/stings encoded with that codepoint. In our case
> everything is converted to utf8 before being returned to the app level. At
> least this is my current understanding of the issue we faced.
> 
> Since I like to work with tests, I made some little changes to the testing
> code in order to be able to run cross compiled tests under windows too. And
> added a case that test specificaly our issue.
> The changes I've made there are not yet providing a fully flavored solution
> (but it is a step forward, that allowed me to test without too much
> troubles).

I like the way you are thinking. We could go one step further and include
a non-standard path in our source tree and access this on all platforms.
This would still require you to set things up the right way when cross
compiling, but would make it one step closer. So add something like

dives/téßtænç/ģïffé₁₂.ssrf

and load that.
 
> What could come next (but I prefer to do this in a separate change) would
> be:
> - Packaging of the tests together with dependencies and test data for
> windows run
>  (currently I went the hard way of manually copying what I needed)

It should be easy enough to have an option to include the tests in the
existing installer - just use a different .nsi to start with, or modify
the .nsi.in to get what you need

> - Making test execution not depending on user name. (All cloud part of
> TestGitStorage implementation uses default path based on user dir to
> execute tests.)

So there are different things to consider here. TestGitStorage should
certainly test the same code that would be run by Subsurface. But you
could add a TestNonAscii which uses the file suggested above and creates
git storage below a similarly creatively named folder.

> - Make test data user independent. (When I run the tests I get many:
> "didn't find picture entry for
> /home/hohndel/subsurface/dives/images/PA102039.jpg"
> caused by hard coded path in the test cases)

Yeah, I noticed this last weekend as well, when running tests on a fresh
VM. That certainly should be fixed in the test data.

> - I still could not run TestPreferences.exe I get a strange missing
> dependency message (certainly explained by my manual copy of the
> dependencies ;))
> - Some issue with recently added TestMerge and TestParse caused by a
> trailing \r in the value from written file
>    Actual   (readin.takeFirst()) : "<divelog program='subsurface'
> version='3'>"
>    Expected (written.takeFirst()): "<divelog program='subsurface'
> version='3'>\r"

We should ignore line endings when comparing the files.

> For now I got TestGitStorage to:
> 1. Reproduce the issue when the fix was not there. see log below, I cut off
> the missing image messages
> 2. Pass once the fix is included. see log below too
> 
> Have a look at the pull request, I will be happy to make any improvements
> or fixes to the change. (I will not be unhappy if you pull it in as is)

I'll look at that next.

Thanks for working on this!

/D