Skip to content

feat(windows): known folders support #259

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

convey-gilbert
Copy link

@convey-gilbert convey-gilbert commented Oct 3, 2018

Access to known user folders:
documentsDirectory: Documents,
picturesDirectory: Pictures,
musicDirectory: Music,
videoDirectory: Videos

Platforms affected

Windows

What does this PR do?

Add support for known folders. Essential feature that is needed to access any file outside of private app sandbox

Limitation:

There is stil a problem, that working on known folders / file libraries like this will only work on Windows 10, not on 8.x
(copied from comment below)

What testing has been done on this change?

Tested on Windows 10 Pro 1803 Build 17134.320

Checklist

  • Reported an issue in the JIRA database
  • Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
  • Added automated test coverage as appropriate for this change.

Access to known user folders:
documentsDirectory: Documents, picturesDirectory: Pictures,
musicDirectory: Music,
videoDirectory: Videos
@janpio
Copy link
Member

janpio commented Oct 3, 2018

Nice!

eslint is not happy though:

/home/travis/build/apache/cordova-plugin-file/www/resolveLocalFileSystemURI.js
  58:18  error  Expected exception block, space or tab after '/*' in comment  spaced-comment
  58:18  error  Expected space or tab before '*/' in comment                  spaced-comment

Has no effect
@janpio
Copy link
Member

janpio commented Oct 3, 2018

Ouch, doesn't like your editor settings:

/home/travis/build/apache/cordova-plugin-file/src/windows/FileProxy.js
  186:2   error  Unexpected tab character                           no-tabs
  186:2   error  Expected indentation of 4 spaces but found 1 tab   indent
  186:58  error  Strings must use singlequote                       quotes
  187:2   error  Unexpected tab character                           no-tabs
  187:3   error  Expected indentation of 4 spaces but found 2 tabs  indent
  188:2   error  Unexpected tab character                           no-tabs
  188:2   error  Expected indentation of 0 spaces but found 1 tab   indent
  189:2   error  Unexpected tab character                           no-tabs
  189:2   error  Expected indentation of 4 spaces but found 1 tab   indent
  189:76  error  Strings must use singlequote                       quotes
  190:2   error  Unexpected tab character                           no-tabs
  190:3   error  Expected indentation of 4 spaces but found 2 tabs  indent
  191:2   error  Expected indentation of 0 spaces but found 1 tab   indent
  191:2   error  Unexpected tab character                           no-tabs
  192:2   error  Unexpected tab character                           no-tabs
  192:2   error  Expected indentation of 4 spaces but found 1 tab   indent
  193:2   error  Unexpected tab character                           no-tabs
  193:3   error  Expected indentation of 4 spaces but found 2 tabs  indent
  194:2   error  Mixed spaces and tabs                              no-mixed-spaces-and-tabs
  194:6   error  Unexpected tab character                           no-tabs
  195:2   error  Unexpected tab character                           no-tabs
  195:3   error  Expected indentation of 0 spaces but found 2 tabs  indent
  196:2   error  Unexpected tab character                           no-tabs
  196:3   error  Expected indentation of 4 spaces but found 2 tabs  indent
  197:2   error  Unexpected tab character                           no-tabs
  197:2   error  Mixed spaces and tabs                              no-mixed-spaces-and-tabs
  198:2   error  Unexpected tab character                           no-tabs
  198:3   error  Expected indentation of 0 spaces but found 2 tabs  indent
  199:2   error  Unexpected tab character                           no-tabs
  199:3   error  Expected indentation of 4 spaces but found 2 tabs  indent
  200:2   error  Mixed spaces and tabs                              no-mixed-spaces-and-tabs
  200:2   error  Unexpected tab character                           no-tabs
  201:2   error  Unexpected tab character                           no-tabs
  201:3   error  Expected indentation of 0 spaces but found 2 tabs  indent
  202:2   error  Unexpected tab character                           no-tabs
  202:3   error  Expected indentation of 4 spaces but found 2 tabs  indent
  203:2   error  Mixed spaces and tabs                              no-mixed-spaces-and-tabs
  203:2   error  Unexpected tab character                           no-tabs
  204:2   error  Unexpected tab character                           no-tabs
  204:3   error  Expected indentation of 0 spaces but found 2 tabs  indent
  205:2   error  Expected indentation of 0 spaces but found 1 tab   indent
  205:2   error  Unexpected tab character                           no-tabs

By the way: There is no real testing going on for Windows itself, we are currently working on fixing that.

PS: You can probably use eslint --fix (or a similar command) to fix that automatically.

@convey-gilbert
Copy link
Author

I can fix this irrelevant formalia you criticize. But beside that, it'll be more important to be concerned of real functionality and usefulness of the module.
There is stil a problem, that working on known folders / file libraries like this will only work on Windows 10, not on 8.x
I don't need to mention the mess with Android and memory cards :/
It might be a much better approach to use a syntax for symbolic path placeholders. At least Documents, Pictures, Music and Videos. And at least for current-user, user: and maybe public/default for systems without multi-user management. Eventual use of memory card should be transparent to the caller.
The pseudo-path will always access the root folder of the file library, sub-folders maybe added with path delimiter /

@janpio
Copy link
Member

janpio commented Oct 4, 2018

I can fix this irrelevant formalia you criticize.

It is not me criticizing, but the eslint tool that we use on all/most Apache Cordova software. I just copied its output from CI over. As this enables us to keep a common code standard in Apache Cordova projects, it is very much relevant though.

It might be a much better approach to use a syntax ...

That sounds pretty sensible. Best open an issue in this repo and describe the changes you propose and the reasoning behind them. Sounds like a bigger change though, and would most probably require a new major release.

@convey-gilbert
Copy link
Author

@janpio: I've now installed VisualLinter and will try to check for the tab/space inconsistencies again.
I've seen a typo, too: Property videoDirectory should be called videosDirectory. I'll change that, too.
I can work on the concept for virtual known folders maybe later. For the moment the extension enables at least on current Windows 10 devices automatic inter-store app and desktop app <--> store app file exchange.
I'll be back at this plugin after other more urgent work on cordova-plugin-clippingcamera, cordova-plugin-media-capture and creating a new plugin for useful interaction with usb/virtual-com-port devices on pc devices first, sorry… ;)

@janpio
Copy link
Member

janpio commented Oct 4, 2018

No worry, as long as you don't forget to fix this up we will be happy ;)

@aximobile
Copy link

Hello,

I've been testing this pull request on the following device: Windows 10 pro 1511 build 10586.1176

It seems like Windows.Storage.UserDataPaths is always undefined.
I did some research and it looks like you need the Windows 10 Fall Creators Update (introduced v10.0.16299.0) to use the UserDataPaths.

Only specific Windows 10 devices can download this update which means that not all Windows 10 devices support this extension(like mine).

Is there any other way to get access to the known folders ?

I'm not a pro in native Windows development so any tips are welcome.

@@ -30,7 +30,7 @@ xmlns:android="http://schemas.android.com/apk/res/android"
<issue>https://issues.apache.org/jira/browse/CB/component/12320651</issue>

<engines>
<engine name="cordova-android" version=">=6.3.0" />
<engine name="cordova-android" version=">=6.0.0" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact we have still build configurations with cordova-android: 6.0.0
We haven't recognized an incompatibility with cordova-plugin-file. Was there a serious reason behind to restrict a higher cordova-android version?

@janpio janpio changed the title Windows known folders support feat(windows): known folders support Jul 4, 2019
Copy link
Member

@janpio janpio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR also needs changes to the README so this is actually discoverable to users: https://github.com/apache/cordova-plugin-file#where-to-store-files

@janpio janpio closed this Jul 4, 2019
@janpio
Copy link
Member

janpio commented Jul 4, 2019

Restart tests.

@janpio janpio reopened this Jul 4, 2019
@janpio
Copy link
Member

janpio commented Jul 4, 2019

CI: Once more please.

@janpio janpio closed this Jul 4, 2019
@janpio janpio reopened this Jul 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants