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

[file_selector] Migrate file_dialog_controller to Dart. #6518

Conversation

eugerossetto
Copy link
Contributor

We're setting the basis for the 1-on-1 migration from c++ to Dart. This PR is intended to collect feedback and validate our approach.

Things we have done:

  • Migrate the file_dialog_controller.cpp class to Dart using the win32 package.
  • Add tests and fake classes where needed.

Things that need validation:

  • Changed some of the signatures:
    • Due to constraints of the ffi Pointers, we used COMObject pointers.
    • Instead of returning an HRESULT, this implementation returns int, cloning the signature of the methods exposed by the API.
  • The GetResults method uses queryInterface, to migrate it to Dart this uses the Win32 package that offers a wrapper over it that allows us to retrieve a pointer to a different supported interface.
  • There is no built-in clone method in Dart. In addition to this, as the default constructor has been rewritten, there is no way of passing a FileDialogController as an input parameter.

Issue: #112212 [file_selector_windows] Migrate CPP implementation to Dart.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Add methods of FileDialogController

Add FileDialogController tests.

Change the location where the free of the pointer is placed.
@adpinola adpinola force-pushed the 112212-migrate-file-dialog-controller-to-dart branch from 44f30a5 to f9ee25f Compare October 14, 2022 17:54
@adpinola adpinola deleted the 112212-migrate-file-dialog-controller-to-dart branch November 2, 2022 12:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant