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

Impl prompts and savefile dialog on Windows #9009

Merged
merged 5 commits into from
Mar 8, 2024

Conversation

JunkuiZhang
Copy link
Contributor

@JunkuiZhang JunkuiZhang commented Mar 7, 2024

Description

This is a part of #8809 , and this PR dose not include open file dialog, as I already saw two PRs impl this.

3.mp4

Release Notes:

  • N/A

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Mar 7, 2024
@JunkuiZhang
Copy link
Contributor Author

JunkuiZhang commented Mar 7, 2024

Potential BUG

The directory here:

fn prompt_for_new_path(&self, directory: &Path) -> Receiver<Option<PathBuf>>

can be empty string "", which causes a runtime crash, which is pretty similar with #8933 (as I described in #8825 ):

let cwd = TERMINAL_DB
                .get_working_directory(item_id, workspace_id)

here the cwd get empty string.

Is this intended? or something buggy behind the scene?

@JunkuiZhang
Copy link
Contributor Author

JunkuiZhang commented Mar 7, 2024

By the way, I edited crates/zed/resources/windows/manifest.xml, and now, any app using gpui without this file will not launch.

And now the compatible version of Windows boosts to Windows Vista+, I believe we are fine here, since the alacritty_terminal that Zed depends on, uses conPTY as backends on Windows, which is only available starting from Windows 10 2018 fall upgrade.

@mikayla-maki mikayla-maki self-assigned this Mar 7, 2024
@mikayla-maki
Copy link
Contributor

mikayla-maki commented Mar 8, 2024

Once this branch is up to date with main, I'd be happy to merge it.

That said, I noticed the styling on the alert window is very different from the styling used in #8919, is there a different API we could be using that looks similar to #8919?

Also, I think that bug is acceptable for now. If we do run into it in the wild, we can just send the receiver None or some such.

@a-usr
Copy link
Contributor

a-usr commented Mar 8, 2024

I noticed the styling on the alert window is very different from the styling used in #8919, is there a different API we could be using that looks similar to #8919?

Those are actuall as far as I can tell the default windows dialogs for opening a file/folder and for saving a file respectively. The drop-down Textbox below the Textbox where the name of the file goes in the "Save as" dialog is supposed to be used to pick the file extension for the file.

Also, is there a reason as for why you are using the FileSaveDialog directly instead of this function in the windows API?

@JunkuiZhang
Copy link
Contributor Author

JunkuiZhang commented Mar 8, 2024

Also, is there a reason as for why you are using the FileSaveDialog directly instead of this function in the windows API?

By microsoft:

Starting with Windows Vista, the Open and Save As common dialog boxes have been superseded by the Common Item Dialog.

And there are many answers in stackoverflow suggest doing so.

@JunkuiZhang
Copy link
Contributor Author

JunkuiZhang commented Mar 8, 2024

That said, I noticed the styling on the alert window is very different from the styling used in #8919, is there a different API we could be using that looks similar to #8919?

If you are saying the visual style difference, it could be the difference that I use light Windows theme, that PR uses dark Windows theme.

Edit:
I cloned #8919 and take a pic as below:
屏幕截图 2024-03-08 204102
I use light theme so I believe it's the theme difference that causes the visual difference

@mikayla-maki
Copy link
Contributor

Ahh, I see. Thanks for telling me about the platform norms @JunkuiZhang and @a-usr

@mikayla-maki mikayla-maki merged commit a550b9c into zed-industries:main Mar 8, 2024
8 checks passed
@JunkuiZhang JunkuiZhang deleted the windows-prompts branch April 10, 2024 18:00
SomeoneToIgnore pushed a commit that referenced this pull request Nov 29, 2024
JaLnYn pushed a commit to JaLnYn/zed that referenced this pull request Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants