Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

Add path_provider plugin #12

Merged
merged 12 commits into from
May 3, 2017
Merged

Conversation

szakarias
Copy link
Contributor

No description provided.

@szakarias szakarias requested review from mit-mit and mravn-google May 2, 2017 12:39

@Override
public void onMethodCall(MethodCall call, Result result) {
if (call.method.equals("getTemporaryDirectory")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we use a switch here?

} else if (call.method.equals("getApplicationDocumentsDirectory")) {
result.success(getPathProviderApplicationDocumentsDirectory());
} else if (call.method.equals("getPlatformVersion")) {
result.success("Android " + android.os.Build.VERSION.RELEASE);
Copy link
Contributor

Choose a reason for hiding this comment

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

Platform version should be removed.

if (snapshot.hasError) {
return new Padding(
padding: const EdgeInsets.all(16.0),
child: new Text('Error: ${snapshot.error}'));
Copy link
Contributor

Choose a reason for hiding this comment

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

[Nit] Might look nicer with a trailing comma after new Text(...). Similarly below.

Then again, it might not. Up to you.

return new Padding(
padding: const EdgeInsets.all(16.0), child: new Text(''));
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps you can extract the new Padding(...) so that appears only once. As in

Widget text;
if (snapshot.hasError)
  text = ...
else if (snapshot.hasData)
  text = ...
else
  text = ...
return new Padding(padding:..., child: text);

Copy link
Contributor

@mravn-google mravn-google left a comment

Choose a reason for hiding this comment

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

LGTM


Widget _buildAppDocumentsDirectory(
BuildContext context, AsyncSnapshot<Directory> snapshot) {
if (snapshot.hasError) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The code here seems to be largely duplicated of the code above. Can the two sections be combined in a single method?

builder: _buildAppDocumentsDirectory),
),
],
),
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, there is duplication above with the same tree structure twice. Wonder if it would be worth it to refactor. Up to you.

/// On Android, this uses the `getCacheDir` API on the context.
Future<Directory> getTemporaryDirectory() async {
final String path = await _channel.invokeMethod('getTemporaryDirectory');
if (path == null)
Copy link
Contributor

Choose a reason for hiding this comment

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

You seem to make this test also in the iOS code? Doing it once should be enough. Didn't check Java side.

Copy link
Member

@mit-mit mit-mit left a comment

Choose a reason for hiding this comment

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

Readme LGTM

@szakarias
Copy link
Contributor Author

@mravn-google could you PTAL at main.dart?

Copy link
Contributor

@mravn-google mravn-google left a comment

Choose a reason for hiding this comment

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

SLGTM

Widget _buildDirectory(
BuildContext context, AsyncSnapshot<Directory> snapshot) {
Text text;

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd drop this empty line to make it more apparent that text is defined by the following if.

}
} else {
text = new Text('');
}
Copy link
Contributor

Choose a reason for hiding this comment

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

new Text(foo) should be const Text(foo) when foo is a constant string.

As an alternatively to the else clause in lines 58-60, you could declare Text text = const Text(''); in line 48.

@szakarias szakarias merged commit 1ed46af into flutter:master May 3, 2017
sigurdm pushed a commit to sigurdm/plugins that referenced this pull request May 15, 2018
Fix a bug with video recording sound in IOS
srburton added a commit to srburton/plugins that referenced this pull request Feb 21, 2020
       FAILURE: Build failed with an exception.

                    * Where:
                    Build file 'C:\Users\srburton\AppData\Roaming\Pub\Cache\hosted\pub.dartlang.org\shared_preferences_macos-0.0.1+6\android\build.gradle' line: 22

                    * What went wrong:
                    A problem occurred evaluating root project 'shared_preferences_macos'.
                    > Failed to apply plugin [id 'com.android.library']
                       > Minimum supported Gradle version is 5.4.1. Current version is 5.1.1. If using the gradle wrapper, try editing the distributionUrl in
                       C:\Users\srburton\AppData\Roaming\Pub\Cache\hosted\pub.dartlang.org\shared_preferences_macos-0.0.1+6\android\gradle\wrapper\gradle-wrapper.properties to gradle-5.4.1-all.zip

                    * Try:
                    Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.

                    * Get more help at https://help.gradle.org

                    BUILD FAILED in 1s
[   +1 ms] Running Gradle task 'assembleAarRelease'... (completed in 2,0s)
[        ] 
           FAILURE: Build failed with an exception.
           
           * Where:
           Build file 'C:\Users\srburton\AppData\Roaming\Pub\Cache\hosted\pub.dartlang.org\shared_preferences_macos-0.0.1+6\android\build.gradle' line: 22
           
           * What went wrong:
           A problem occurred evaluating root project 'shared_preferences_macos'.
           > Failed to apply plugin [id 'com.android.library']
              > Minimum supported Gradle version is 5.4.1. Current version is 5.1.1. If using the gradle wrapper, try editing the distributionUrl in
C:\Users\srburton\AppData\Roaming\Pub\Cache\hosted\pub.dartlang.org\shared_preferences_macos-0.0.1+6\android\gradle\wrapper\gradle-wrapper.properties to gradle-5.4.1-all.zip
           
           * Try:
           Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.
           
           * Get more help at https://help.gradle.org
           
           BUILD FAILED in 1s
           
[   +5 ms] "flutter apk" took 179.497ms.
The plugin shared_preferences_macos could not be built due to the issue above.

#0      throwToolExit (package:flutter_tools/src/base/common.dart:28:3)
flutter#1      buildPluginsAsAar (package:flutter_tools/src/android/gradle.dart:726:7)
flutter#2      _asyncErrorWrapperHelper.<anonymous closure> (dart:async-patch/async_patch.dart:80:45)
#3      _rootRunBinary (dart:async/zone.dart:1146:38)
flutter#4      _CustomZone.runBinary (dart:async/zone.dart:1039:19)
flutter#5      _FutureListener.handleError (dart:async/future_impl.dart:153:20)
flutter#6      Future._propagateToListeners.handleError (dart:async/future_impl.dart:692:47)
flutter#7      Future._propagateToListeners (dart:async/future_impl.dart:713:24)
flutter#8      Future._completeError (dart:async/future_impl.dart:532:5)
flutter#9      _AsyncAwaitCompleter.completeError (dart:async-patch/async_patch.dart:38:15)
flutter#10     buildGradleAar (package:flutter_tools/src/android/gradle.dart)
flutter#11     _asyncThenWrapperHelper.<anonymous closure> (dart:async-patch/async_patch.dart:73:64)
flutter#12     _rootRunUnary (dart:async/zone.dart:1134:38)
flutter#13     _CustomZone.runUnary (dart:async/zone.dart:1031:19)
flutter#14     _FutureListener.handleValue (dart:async/future_impl.dart:139:18)
flutter#15     Future._propagateToListeners.handleValueCallback (dart:async/future_impl.dart:680:45)
flutter#16     Future._propagateToListeners (dart:async/future_impl.dart:709:32)
#17     Future._completeWithValue (dart:async/future_impl.dart:524:5)
flutter#18     _AsyncAwaitCompleter.complete (dart:async-patch/async_patch.dart:32:15)
flutter#19     _completeOnAsyncReturn (dart:async-patch/async_patch.dart:290:13)
flutter#20     _DefaultProcessUtils.run (package:flutter_tools/src/base/process.dart)
flutter#21     _asyncThenWrapperHelper.<anonymous closure> (dart:async-patch/async_patch.dart:73:64)
flutter#22     _rootRunUnary (dart:async/zone.dart:1134:38)
flutter#23     _CustomZone.runUnary (dart:async/zone.dart:1031:19)
flutter#24     _FutureListener.handleValue (dart:async/future_impl.dart:139:18)
flutter#25     Future._propagateToListeners.handleValueCallback (dart:async/future_impl.dart:680:45)
flutter#26     Future._propagateToListeners (dart:async/future_impl.dart:709:32)
flutter#27     Future._completeWithValue (dart:async/future_impl.dart:524:5)
flutter#28     Future.wait.<anonymous closure> (dart:async/future.dart:400:22)
flutter#29     _rootRunUnary (dart:async/zone.dart:1134:38)
flutter#30     _CustomZone.runUnary (dart:async/zone.dart:1031:19)
flutter#31     _FutureListener.handleValue (dart:async/future_impl.dart:139:18)
flutter#32     Future._propagateToListeners.handleValueCallback (dart:async/future_impl.dart:680:45)
flutter#33     Future._propagateToListeners (dart:async/future_impl.dart:709:32)
flutter#34     Future._completeWithValue (dart:async/future_impl.dart:524:5)
flutter#35     Future._asyncComplete.<anonymous closure> (dart:async/future_impl.dart:554:7)
flutter#36     _rootRun (dart:async/zone.dart:1126:13)
flutter#37     _CustomZone.run (dart:async/zone.dart:1023:19)
flutter#38     _CustomZone.runGuarded (dart:async/zone.dart:925:7)
flutter#39     _CustomZone.bindCallbackGuarded.<anonymous closure> (dart:async/zone.dart:965:23)
flutter#40     _microtaskLoop (dart:async/schedule_microtask.dart:43:21)
flutter#41     _startMicrotaskLoop (dart:async/schedule_microtask.dart:52:5)
flutter#42     _runPendingImmediateCallback (dart:isolate-patch/isolate_patch.dart:118:13)
flutter#43     _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:175:5)
@srburton srburton mentioned this pull request Feb 21, 2020
13 tasks
@muhammednazil muhammednazil mentioned this pull request Dec 20, 2020
nt4f04uNd pushed a commit to nt4f04uNd/plugins that referenced this pull request Mar 23, 2021
…h-feature

Added test coverage to flash feature
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants