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

Open Model Folder #167

Merged
merged 4 commits into from
Jan 30, 2024
Merged

Open Model Folder #167

merged 4 commits into from
Jan 30, 2024

Conversation

feloy
Copy link
Contributor

@feloy feloy commented Jan 29, 2024

Fixes partially #50

function openModelFolder() {
console.log(object.file);
if (object && object.file) {
studioClient.openURL('file://'+object.file.path);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would use uris everywhere

build the URI using

Uri.file(object.file.path)

and then not have in studioClient

  async openURL(url: string): Promise<boolean> {
    return await podmanDesktopApi.env.openExternal(podmanDesktopApi.Uri.parse(url));
  }

but just accept URI (to avoid double parse/unexpected behavior)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I did not pay enough attention to backend/frontend usage so you may disregard my comment as basically frontend has no access to Podman Desktop API and the URI class.

so either you revert the commit or you have openFile(filePath: string) method on studio-client that will do openExternal(URI.file(filePath))

sorry for the noise

@feloy feloy force-pushed the feat-50/opn-model-folder branch 3 times, most recently from 0c4f13b to 74cbcb4 Compare January 29, 2024 13:09
@feloy feloy requested a review from benoitf January 29, 2024 13:11
Copy link
Contributor

@lstocchi lstocchi left a comment

Choose a reason for hiding this comment

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

When opening the model folder i see this dialog but the title and text are wrong. I'm not opening any external website

image

Copy link
Contributor

@jeffmaury jeffmaury left a comment

Choose a reason for hiding this comment

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

LGTM

@axel7083
Copy link
Contributor

I had #122 open, I will put it as draft waiting for this one to be merged and reuse your code @feloy

@feloy
Copy link
Contributor Author

feloy commented Jan 29, 2024

When opening the model folder i see this dialog but the title and text are wrong. I'm not opening any external website

image

I created an issue on Podman Desktop: podman-desktop/podman-desktop#5732

@feloy feloy requested a review from lstocchi January 29, 2024 15:36
Copy link
Contributor

@lstocchi lstocchi left a comment

Choose a reason for hiding this comment

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

I would fix the issue on desktop before releasing tbh but maybe @jeffmaury wants to have it for the release.

BTW looking at it again ... do we really need to show that dialog confirmation? We are just opening a local folder, nothing too serious. Can we just open the folder without asking to confirm?

@jeffmaury
Copy link
Contributor

I would fix the issue on desktop before releasing tbh but maybe @jeffmaury wants to have it for the release.

BTW looking at it again ... do we really need to show that dialog confirmation? We are just opening a local folder, nothing too serious. Can we just open the folder without asking to confirm?

That maybe the cause of security issue, @benoitf WDYT ?

@benoitf
Copy link
Collaborator

benoitf commented Jan 29, 2024

I created the commit and the branch but forgot to open the PR

here it is podman-desktop/podman-desktop#5743

@feloy feloy force-pushed the feat-50/opn-model-folder branch from 74cbcb4 to 87353db Compare January 30, 2024 08:45
@feloy feloy merged commit 150452d into main Jan 30, 2024
3 checks passed
@feloy feloy deleted the feat-50/opn-model-folder branch January 30, 2024 09:28
mhdawson pushed a commit to mhdawson/podman-desktop-extension-ai-lab that referenced this pull request Nov 22, 2024
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.

5 participants