Skip to content
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

Respect starting directory for dialogs on Windows #1452

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,4 @@ Andrey Kabylin
Garrett Risley
Robert Wittams
Jaap Aarts
Maximilian Köstler
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ You can find its changes [documented below](#060---2020-06-01).
- `WidgetPod::is_initialized` to check if a widget has received `WidgetAdded`. ([#1259] by [@finnerale])
- `TextBox::with_text_alignment` and `TextBox::set_text_alignment` ([#1371] by [@cmyr])
- Add default minimum size to `WindowConfig`. ([#1438] by [@colinfruit])
- Windows: Dialogs now respect the parameter passed to `force_starting_directory()` ([#1452] by [@MaximilianKoestler])

### Changed

Expand Down Expand Up @@ -362,6 +363,7 @@ Last release without a changelog :(
[@colinfruit]: https://github.com/colinfruit
[@Maan2003]: https://github.com/Maan2003
[@derekdreery]: https://github.com/derekdreery
[@MaximilianKoestler]: https://github.com/MaximilianKoestler

[#599]: https://github.com/linebender/druid/pull/599
[#611]: https://github.com/linebender/druid/pull/611
Expand Down Expand Up @@ -550,6 +552,7 @@ Last release without a changelog :(
[#1438]: https://github.com/linebender/druid/pull/1438
[#1441]: https://github.com/linebender/druid/pull/1441
[#1448]: https://github.com/linebender/druid/pull/1448
[#1452]: https://github.com/linebender/druid/pull/1452

[Unreleased]: https://github.com/linebender/druid/compare/v0.6.0...master
[0.6.0]: https://github.com/linebender/druid/compare/v0.5.0...v0.6.0
Expand Down
26 changes: 26 additions & 0 deletions druid-shell/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright 2020 The Druid Authors.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think a more standard way to do this would be to add the "shellapi" feature to winapi-rs, rather than adding a build.rs here.

Copy link
Contributor Author

@MaximilianKoestler MaximilianKoestler Dec 13, 2020

Choose a reason for hiding this comment

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

Well... that seems to be a much more reasonable idea than what I have done. It was a bit foolish by me to assume that there is no solution for this in the winapi-crate.

Before I add "shellapi" to the list, do you have an idea if there is a specific meaning to how the features of winapi are ordered and formatted?
When I see a block like this in a Cargo.toml, I always wonder where to add something new and where to put the line breaks afterwards:

features = ["d2d1_1", "dwrite", "winbase", "libloaderapi", "errhandlingapi", "winuser",
            "shellscalingapi", "shobjidl", "combaseapi", "synchapi", "dxgi1_3", "dcomp",
            "d3d11", "dwmapi", "wincon", "fileapi", "processenv", "winbase", "handleapi"]

Copy link
Member

Choose a reason for hiding this comment

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

@MaximilianKoestler while I admire your diligence, I don't think there is a clear ordering. For line breaks, a reasonable guideline is to just keep us at 80 columns. In a perfect world ordering would be alphabetical and fixed by rustfmt, but I think you should just add yours at the end (probably after a newline)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cmyr Thanks. I know that I am the type of dev who usually thinks about these kinds of things for one minute too long 😄. I will just go with the smallest delta.

While I am at it: In this project, do you prefer

  • changes after review as a separate commit,
  • changes after review as a separate commit, but squashed during PR merge,
  • or just force pushing on the PR branch?

Copy link
Member

Choose a reason for hiding this comment

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

We are decidedly anarchic. For a small patch, force pushing is fine; at merge time if commits are clearly written or clearly intentional in their structure I will rebase, and if things are more messy I will squash.

//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

//! Build script to modify the build environment

use std::env;

fn main() {
let env = env::var("TARGET").unwrap();
if env.contains("windows") {
// Since https://github.com/rust-lang/rust/pull/56568, shell32 is not included in all
// Windows binaries by default.
println!("cargo:rustc-link-lib=shell32");
}
}
16 changes: 16 additions & 0 deletions druid-shell/src/platform/windows/dialog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use std::convert::TryInto;
use std::ffi::OsString;
use std::ptr::null_mut;

use winapi::ctypes::c_void;
use winapi::shared::minwindef::*;
use winapi::shared::ntdef::LPWSTR;
use winapi::shared::windef::*;
Expand Down Expand Up @@ -147,6 +148,21 @@ pub(crate) unsafe fn get_file_dialog_path(

as_result(file_dialog.SetOptions(flags))?;

// set a starting directory
if let Some(path) = options.starting_directory {
let mut item: *mut IShellItem = null_mut();
if let Err(err) = as_result(SHCreateItemFromParsingName(
path.as_os_str().to_wide().as_ptr(),
null_mut(),
&IShellItem::uuidof(),
&mut item as *mut *mut IShellItem as *mut *mut c_void,
)) {
log::warn!("Failed to convert path: {}", err.to_string());
} else {
as_result(file_dialog.SetDefaultFolder(item))?;
}
}

// show the dialog
as_result(file_dialog.Show(hwnd_owner))?;
let mut result_ptr: *mut IShellItem = null_mut();
Expand Down