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

add support for absolute sqlite db path in case of using tauri dialog… #609

Closed

Conversation

syokensyo
Copy link

Since tauri provide dialog api, we can use it to browse files on the computer and get the file's absolute path, it is more flexible to make the sqlite plugin adapt to that, so user can operate on any sqlite db file in their computer, no need to put db file to the app's path anymore.

@syokensyo syokensyo requested a review from a team as a code owner September 21, 2023 07:20
@FabianLars
Copy link
Member

Thanks for contributing! Before anything else, to be able to merge this PR your commit must be signed :)

Now about the change itself: i also worked on this half a year ago here: #279.
I'm afraid that the implementation can't be as simple as in your PR since we have to support both / and \ on all OSes, have to handle the stupid UAC path on Windows (the \\?\ path prefix) and may or may not have to handle sqlite: and sqlite:// at the same time since both are valid - in my PR i just added comments that only sqlite:// is supported but not sure if i'm 100% happy with this decision.

@syokensyo
Copy link
Author

syokensyo commented Sep 21, 2023

Oh, sorry, didn't notice your PR, and yes you are right, I didn't cover enough scenarios, will look into your pr, and I think absolute path support is a frequency requirement for sqlite user, it's better to support limited scenarios than no scenario.

@amrbashir
Copy link
Member

you could use dunce crate which will remove the UNC on Windows and keep the path as is on other platforms.

@syokensyo
Copy link
Author

Will try it, thanks.

@syokensyo syokensyo closed this Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants