Add rename support for Windows

[brianquinlan]

• I’ve reviewed the contributor guide and applied the relevant portions to this PR.

Contribution guidelines:

• See our contributor guide for general expectations for PRs.
• Larger or significant changes should be discussed in an issue before creating a PR.
• Contributions to our repos should follow the Dart style guide and use dart format.
• Most changes should add an entry to the changelog and may need to rev the pubspec package version.
• Changes to packages require corresponding tests.

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

[devoncarew]

Some thoughts on the PR but nothing blocking.

[devoncarew]

Perhaps:

FileSystem get fileSystem => Platform.isWindows ? WindowsFileSystem() : PosixFileSystem();

I assume creating these is cheap?

[brianquinlan]

Yep. These classes have no state.

[devoncarew]

above, at the start of main(), perhaps assign the filesystem to a local? final fs = fileSystem;.

[brianquinlan]

Any reason why? Are you concerned with performance?

[devoncarew]

No, mostly for documentation purposes in terms of what filesystem is being tested.

[brianquinlan]

Sorry, this isn't quite ready for review but I forgot to press the "draft" button.

[brianquinlan]

@devoncarew Do you want to take another look?

[devoncarew]

I'm curious - the web file systems will have differences between the OS platforms?

[brianquinlan]

I'm not sure what API design is right but I want the user to be able to write something like:

bool isHidden(String path) {
  if (fileSystem is WindowsFileSystem) {
    return fileSystem.metadata(path).isHidden;
  } else {
    return path.startsWith(".");
  }
}

Either package:io_file has to protect the web developer from ffi imports or the developer will have to do that themselves (like they do have to with package:http: https://github.com/dart-lang/http/blob/9129a96be871a5779436f042bd13790960171373/pkgs/flutter_http_example/lib/main.dart#L13).

[devoncarew]

No, mostly for documentation purposes in terms of what filesystem is being tested.

[halildurmus]

The memory allocated for these two paths must be freed manually or by wrapping this function's body with using and passing arena as the allocator argument in toNativeUtf16:

void rename(String oldPath, String newPath) {
  using((arena) {
    // ...
    if (win32.MoveFileEx(
          oldPath.toNativeUtf16(allocator: arena),
          newPath.toNativeUtf16(allocator: arena),
    // ...

[brianquinlan]

Done, and great drive by code review! Do you want to review more windows file system code?

[halildurmus]

Done, and great drive by code review!

Thanks!

Do you want to review more windows file system code?

Sure! Feel free to send review requests in future PRs. I'd be happy to review them.