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

terminal profiles broken again #3275

Closed
maddes8cht opened this issue May 19, 2024 · 10 comments
Closed

terminal profiles broken again #3275

maddes8cht opened this issue May 19, 2024 · 10 comments
Labels
C-bug Category: bug - something isn't working as it's supposed to
Milestone

Comments

@maddes8cht
Copy link

Lapce Version

nightly -11f36a7

System information

Windows 10

Describe the bug

With the current nightly, the formerly closed issue #3244 is back again and worse than before:

  • Not only does the before working settings.toml i posted in Terminal Settings Behaviour Not as Expected - TOML Parser Issue and Start Behaviour #3244 stop working again (again it always opens powershell)
  • in the settings, section terminal, in the Terminal: default profile there is only defaultavailable, the other
    configured terminals have disappeared.
  • when i change any settings in the graphical interface, the settings.toml gets shot in the foot (in different ways, see aditional information)

Additional information

The last item in my list is very inconsistent and not very reproducible, thus i don't have screenshots or .toml file content for this.
I experienced once that the automatically written settings.toml was garbaged in the way that almost all closing bracket like ] , } and even closing " had vanished, causing lapce to be stalled when loading.
I also experienced when changing the core: Color Theme setting that i got a settings.tomlwith only the setting for the core: Color Theme, all other settings had been deleted. This seems more reproducible.

@maddes8cht maddes8cht added the C-bug Category: bug - something isn't working as it's supposed to label May 19, 2024
@panekj
Copy link
Collaborator

panekj commented May 26, 2024

Can't reproduce

@RivenSkaye
Copy link

I can reproduce this - on latest stable even. Though slightly different from the original issue because it's only the first tab. If I hit the + I do get an msys profile terminal.
image
image
image

term config:

[terminal]
font-family = "'Hasklug NF', 'Hasklug Nerd Font'"
font-size   = 14

[terminal.default-profile]
windows = "msys"

[terminal.profiles.msys]
args    = ["--login", "-i"]
command = "C:/msys64/usr/bin/bash.exe"
env     = {MSYSTEM = "MINGW64", CHERE_INVOKING = "1", MSYS2_PATH_TYPE = "inherit", MSYS2_NOSTART = "yes", MSYSCON = "defterm"}

@maddes8cht you might wanna test for this as well. Though granted, it kinda sucks to have to open two shells to get the one you want.
You might want to try defterm to ensure it hooks into the calling console host rather than opening its own. nostart shouldn't be needed (it's only relevant when starting through the cmd script AFAICT)

@panekj
Copy link
Collaborator

panekj commented May 27, 2024

I can reproduce this - on latest stable even

An issue that wasn't fixed in stable..

@RivenSkaye
Copy link

I was merely surprised a commit causing an issue in nightly that wasn't noted until a few days before the stable release got through; I figured the time frame would be larger than the few days between this issue and the 0.4.0 release. If I were to be mad about issues in stable, there's be lower hanging fruit.

Small update on the issue at hand: <msys makes it open the correct terminal out of view, then clicking the term tab in the bottom panel doesn't cause a powershell session to be started. It does not, unfortunately, bring the terminal into view immediately. So I think something in the terminal tab init is skipping settings. Closing the last terminal and reopening the term tab consistently brings back powershell if that could help for repro

@panekj
Copy link
Collaborator

panekj commented May 27, 2024

term config:

[terminal]
font-family = "'Hasklug NF', 'Hasklug Nerd Font'"
font-size   = 14

[terminal.default-profile]
windows = "msys"

[terminal.profiles.msys]
args    = ["--login", "-i"]
command = "C:/msys64/usr/bin/bash.exe"
env     = {MSYSTEM = "MINGW64", CHERE_INVOKING = "1", MSYS2_PATH_TYPE = "inherit", MSYS2_NOSTART = "yes", MSYSCON = "defterm"}

this config is invalid btw.
https://docs.lapce.dev/get-started/terminal

@RivenSkaye
Copy link

I can't exactly tell what's invalid about that config tbh; what would be breaking about it?

@panekj
Copy link
Collaborator

panekj commented May 27, 2024

image
image

@RivenSkaye
Copy link

Changing those to match the keys as used in the example does indeed impact the opened env, and removing CHERE_INVOKING also makes msys respect the set workdir. But still only on the second and later tabs.

@RivenSkaye
Copy link

Okay so I did manage to find the code path triggering the wrong profile being loaded on clean terminal pane tabs.
show_panel initializes the first term with a static None
This is propagated through TerminalPanelData -> TerminalTabData -> TerminalData where it goes through a few methods to end up being an arg in this call to Self::new_raw_terminal which immediately shadows profile with unwrap_or_default() to ensure a profile struct is instantiated.

After that I'm hitting a dead end, as the defaults should result in an eventual situation of creating a console host with no target program, which I think should crash or spin up cmd.exe on w10? Save for Windows Terminal I don't know if anything explicitly defaults to or falls back on powershell or if that was changed recently. And searching the code doesn't help as Lapce makes very few mentions of any default shells.

@panekj
Copy link
Collaborator

panekj commented May 28, 2024

Fixed via af72732

@panekj panekj closed this as completed May 28, 2024
@panekj panekj added this to the Next Release milestone Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug - something isn't working as it's supposed to
Projects
None yet
Development

No branches or pull requests

3 participants