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.

Sign up for GitHub

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

Jump to bottom

Display a specific error for not finding the default profile #1348

Closed
3 tasks
zadjii-msft opened this issue Jun 20, 2019 · 3 comments · Fixed by #2422
Closed
3 tasks

Display a specific error for not finding the default profile #1348

zadjii-msft opened this issue Jun 20, 2019 · 3 comments · Fixed by #2422
Assignees
@zadjii-msft
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Terminal 1908.1

Comments

@zadjii-msft
Copy link
Member

zadjii-msft commented Jun 20, 2019 •
edited
Loading

From #1343

Even more correct would be to specifically display a message that there was no default profile (though that might be too much for this release).

  • If we load the profiles.json and the defaultProfile doesn't exist in the list of profiles, we should display an error specific to that failure case.
  • We should also use the first profile as the default temporarily. We'll need to make sure not to persist this runtime change.

Maybe also:

  • Check if the commandline is a path to a single executable, and if it is, warn if it doesn't exist? Dunno how we'd get the first arg from the commandline easily but it could totally be done.
@zadjii-msft zadjii-msft added Area-User Interface Issues pertaining to the user interface of the Console or Terminal Product-Terminal The new Windows Terminal. Issue-Task It's a feature request, but it doesn't really need a major design. labels Jun 20, 2019
@zadjii-msft zadjii-msft added this to the Terminal v1.0 milestone Jun 20, 2019
@ghost ghost added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jun 20, 2019
@DHowett-MSFT DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jun 20, 2019
This was referenced Jun 24, 2019
Closed
Closed
@karypid
Copy link

karypid commented Jul 2, 2019

I would like to add that this should also happen if the default profile is present but fails to load.

So, even if profiles.json is correct and default profile actually exists in it, but the command runs and exits in less than 5 seconds, you should get a dialog saying "command exited almost immediately, please check profiles.json to ensure it is correct".

This is similar to what OS X's Terminal application and the popular Windows "ConEmu" application already do.

I actually came across this issue in a nasty way: when I installed Windows Terminal on a new laptop, Windows Store downloaded the profiles.json file from my previous laptop. When I launched the application the window appeared and immediately disappeared without giving me a chance to read the output. What was happening is that I had edited the command for my default profile and set it to:

            "commandline" : "powershell.exe -noexit -file env.ps1",

Now, normally in my home folder I have a file "env.ps1" where I define some functions, aliases etc. Because the new laptop did NOT have this file yet, powershell would exit immediately with an error.

This was referenced Jul 31, 2019
Closed
Closed
Merged
@miniksa miniksa modified the milestones: Terminal v1.0, Terminal 1908.1 Aug 1, 2019
@miniksa
Copy link
Member

miniksa commented Aug 1, 2019

Please at least minimally stop the crash from happening when the default is gone. Choose a different profile and try to recover.

@miniksa miniksa mentioned this issue Aug 1, 2019
Closed
zadjii-msft added a commit that referenced this issue Aug 6, 2019
@zadjii-msft
Warn the user when their settings are bad
80c18fb 
  The start of work on #1348
This was referenced Aug 8, 2019
Closed
Merged
@ghost ghost added the In-PR This issue has a related PR label Aug 13, 2019
@ghost ghost closed this as completed in #2422 Aug 16, 2019
ghost pushed a commit that referenced this issue Aug 16, 2019
@zadjii-msft
Add Warnings during settings load (#2422)
d7d96f7 
* Warn the user when their settings are bad

  The start of work on #1348

* Display an error dialog for errors during validation

* Polish for PR

  * Add a ton of tests
  * Polish the _GetMessageText bits
  * Add code to check for duplicate profiles
  * Verify that many warnings work at the same time
  * comments y'all

* Apply fixes for dustin's thoughts from PR

* Add a proper exception type, use an array instead of a map

* PR Fixes

  * Fix x86 build break
  * Add a bit on "using the defaults" when we encountering an exception
  * remove a redundant variable

* guid->GUID

* Address Michael's PR comments

* Clean up this error text, and catch exceptions better

* Update src/cascadia/TerminalApp/Resources/en-US/Resources.resw
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Aug 16, 2019
@ghost
Copy link

ghost commented Aug 27, 2019

🎉This issue was addressed in #2422, which has now been successfully released as Windows Terminal Preview v0.4.2382.0.:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zadjii-msft zadjii-msft

Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
Terminal 1908.1
Development

Successfully merging a pull request may close this issue.

Add Warnings during settings load microsoft/terminal
4 participants
@karypid @DHowett-MSFT @miniksa @zadjii-msft