Skip to content

Implement file_chooser for Windows #651

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

Merged
merged 5 commits into from
Jan 27, 2020

Conversation

stuartmorgan-g
Copy link
Collaborator

Part of #105


// Sets the filter for allowed file types to select.
void SetAllowedExtensions(const EncodableList &extensions) {
const std::wstring name_delimiter = L", ";
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to have a comment explaining what the final structure looks like.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

}
}

// Attempts to set the default folder for the dialog to |path|,
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we using /// for documentation comments?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We haven't been, and it's not a standard Google C++ style guide style.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I've seen it in the engine's headers so maybe I got confused from there.

filter += file_wildcard + extension;
}
// TODO: Make a meaningful filterspec array instead of one mega-filter.
// See issue #650.
Copy link
Contributor

Choose a reason for hiding this comment

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

Which issue is this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

#650 ? I'm not sure what you're asking exactly since the issue number is in the comment...

Copy link
Contributor

Choose a reason for hiding this comment

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

lol I was looking for 650 in the engine's repo and just didn't make any sense 😕

EncodableValue Show(HWND parent_window) {
assert(dialog_);
last_result_ = dialog_->Show(parent_window);
last_result_ == HRESULT_FROM_WIN32(ERROR_CANCELLED);
Copy link
Contributor

Choose a reason for hiding this comment

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

what does this line do?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tells us that either we need better warning settings, or I wasn't watching the warnings list :) It's cruft from some restructuring when I realized I didn't need a different codepath for cancelled here that used to be a condition. I'm not sure how I accidentally made it into a statement.

Removed.

@nadimgouia
Copy link

after adding the plugin to project, is there any special configuration to make it work on windows ?
i mean by configuration something like adding those lignes for macos :

<key>com.apple.security.files.user-selected.read-only</key>
    <true/>

@stuartmorgan-g
Copy link
Collaborator Author

after adding the plugin to project, is there any special configuration to make it work on windows ?

I'm not aware of any such requirements, which is why this PR doesn't update the README.

@stuartmorgan-g stuartmorgan-g merged commit 52da38c into google:master Jan 27, 2020
@stuartmorgan-g stuartmorgan-g deleted the windows-file-chooser branch January 27, 2020 17:14
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 5, 2021
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