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

New-EditorFile works on non-powershell untitled files #1580

Merged
merged 6 commits into from
Oct 29, 2018

Conversation

TylerLeonhardt
Copy link
Member

Partnered with: PowerShell/PowerShellEditorServices#774

This fixes the problem where $psEditor.GetEditorContext() assumed that the file open was a real file on the file system OR a powershell untitled file. Since that isn't always the case, It would throw an exception:

Exception calling "GetEditorContext" with "0" argument(s): "One or more errors occurred. (Could not find file '/Users/tyler/Code/PowerShell/vscode/PowerShellEditorServices/untitled:Untitled-1'.)"
At /Users/tyler/.vscode-insiders/extensions/ms-vscode.powershell-1.9.0/modules/PowerShellEditorServices/Commands/Public/CmdletInterface.ps1:152 char:13
+             $psEditor.GetEditorContext().CurrentFile.InsertText(($val ...
+             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo          : NotSpecified: (:) [], MethodInvocationException
+ FullyQualifiedErrorId : AggregateException

This also means that New-EditorFile -Value 'asdf' actually works when you don't have your default language mode set to PowerShell.

This PR was needed to give GetEditorContext the content of the untitled file.

fixes #810

Copy link
Contributor

@rkeithhill rkeithhill left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@SeeminglyScience SeeminglyScience left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@rjmholt rjmholt left a comment

Choose a reason for hiding this comment

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

LGTM

corbob and others added 5 commits October 15, 2018 14:13
…1555)

* Log when languageClient not loaded.
Copy the pattern from ExtensionsCommandsFeature to the other instances of the check without a log of the error.
Make the PSScriptAnalyzer issue redirect template more explicit about what PSScriptAnalyzer handles in the PowerShell extension.
@rjmholt
Copy link
Contributor

rjmholt commented Oct 24, 2018

I'm assuming the branch is ok here? Seems to have incorporated commits from other PRs somehow...

@TylerLeonhardt
Copy link
Member Author

Yeah that was from a rebase I think

@SeeminglyScience
Copy link
Collaborator

@tylerl0706 my understanding is that if you rebase a branch that is already published you need to force push for it to be clean. That does rewrite history, but it's probably fine for a fork. You can also do a merge instead which will group it probably.

That said I'm not a git ninja so ymmv 😉

@TylerLeonhardt
Copy link
Member Author

Yeah looking at the history (because I forgot lol), that's what I did (a rebase) which is what I wanted.

Had to force push to rewrite history but I wanted that because I wanted my commits in this PR to be after what's in master.

I think it's typically a good idea to rebase before merging - especially when the target branch had a lot of action - just to make sure your stuff works with what's currently in the target.

A merge from target to incoming works fine too... But I like the idea of my PR commits being after what's in the target

@SeeminglyScience
Copy link
Collaborator

@tylerl0706 are you sure you force pushed though? That looks like a normal push after rebase (the button in VSCode is a normal push)

@TylerLeonhardt
Copy link
Member Author

Yeah I'm pretty sure I did. I typically use the git CLI. I'll double check though

@TylerLeonhardt
Copy link
Member Author

Yep the log looks good and I even did a force push again and that said everything is up to date.

10-24 10:01:18 ~/Code/PowerShell/vscode/vscode-powershell [fix-new-editorfile]
→ git log
commit 0435f8f6ee485d0424e6de90eb955a25b01997e6 (HEAD -> fix-new-editorfile, personal/fix-new-editorfile)
Merge: c4db8d7 6d46bdc
Author: Tyler Leonhardt <tylerl0706@gmail.com>
Date:   Sun Oct 21 08:48:25 2018 +0800

    merge conflict

commit c4db8d7c78c994cfbb78b77142ec059145c08617
Author: Tyler Leonhardt <tylerl0706@gmail.com>
Date:   Sun Oct 21 08:46:46 2018 +0800

    add currentFileLanguage

commit c386369b18058c2fcfe75c6fb11573f05f1447df
Author: Tyler Leonhardt <tylerl0706@gmail.com>
Date:   Sun Oct 14 08:18:29 2018 +0700

    New-EditorFile works on non-powershell untitled files

commit 0388343e13e4baadfffc34b25d6d49a6200fee58 (origin/master, origin/HEAD, master)
Author: Robert Holt <rjmholt@gmail.com>
Date:   Mon Oct 15 18:44:00 2018 -0700

    [Ignore] Update PSScriptAnalyzer issue template (#1585)

    Make the PSScriptAnalyzer issue redirect template more explicit about what PSScriptAnalyzer handles in the PowerShell extension.

commit 55e8ca33cae05bc0ac8dc37818094d3dded692e4
Author: corbob <30301021+corbob@users.noreply.github.com>
Date:   Mon Oct 15 14:13:32 2018 -0700

    Log when languageClient not loaded during initialization (#1555)

    * Log when languageClient not loaded.
    Copy the pattern from ExtensionsCommandsFeature to the other instances of the check without a log of the error.

commit 6d46bdc813a2bbca704659b175e63375fc4cdac6
Author: Tyler Leonhardt <tylerl0706@gmail.com>
Date:   Sun Oct 14 08:18:29 2018 +0700
10-24 10:01:38 ~/Code/PowerShell/vscode/vscode-powershell [fix-new-editorfile]
→ git status
On branch fix-new-editorfile
nothing to commit, working tree clean
10-24 10:01:40 ~/Code/PowerShell/vscode/vscode-powershell [fix-new-editorfile]
→ git push personal fix-new-editorfile -f
Everything up-to-date

@rjmholt rjmholt merged commit d02adc5 into PowerShell:master Oct 29, 2018
@TylerLeonhardt TylerLeonhardt deleted the fix-new-editorfile branch August 10, 2020 15:04
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.

$pseditor.GetEditorContext().CurrentFile.GetTextLines() throws on Untitled (never saved) file
6 participants