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

Default to standard code behaviour when outside a container #20

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

crispyricepc
Copy link

No description provided.

Copy link
Owner

@owtaylor owtaylor left a comment

Choose a reason for hiding this comment

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

Generally this makes a lot of sense. Thanks!

The actual exec needs to happen later - after we've prompted the user to install com.visualstudio.code - I think that means that the code should be structured as:

if [ -f /run/.containerenv ] ; then
    flatpak=....
    container_name=...
else
    container_name=""
fi

<prompt for install>

if [ "$container_name" = "" ] ; then
   verbose "Not in a toolbox, running Visual Studio Code directly"
   exec ...
fi

@guerra08
Copy link

I belive the verification if the user is on the host or inside a container should happen before the install prompt. Since the installation is checked with flatpak-spawn, it will fail on the host even if VSCode flatpak is already installed.

After this verification is done, we can check for the install, and later check if we should execute the VSCode flatpak on the host simply with flatpak run.

@velsinki
Copy link

Is this MR still active?
I have a working branch at 1886024 that I believe correctly implements the requested changes. I could make an MR for that, but I don't want to steal @crispyricepc's thunder :-)

@crispyricepc
Copy link
Author

Lol I completely forgot about this. Feel free to close

@velsinki
Copy link

Haha, okay. I am not entirely sure if @owtaylor is still working on this repo though?...

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.

4 participants