Delete projects we wont be keeping around and get pses.vscode working again #1046

TylerLeonhardt · 2019-09-30T06:23:39Z

wowie

ok. so.

THE DELETIONS (aka commit 1)

This deletes:
PowerShellEditorServices
PowerShellEditorServices.Protocol
PowerShellEditorServices.Host

(deletion of test projects will happen after the tests are moved over)

PSES.VSCODE IS A BINARY MODULE (aka commit 2)

I had to refactor some stuff to get PSES.VSCode working. The biggest problem was that I deleted the concept of a "ComponentRegistry" which allowed modules to add "components" to $psEditor.Components. Instead of going that route, I now give modules direct access to the IServiceProvider that contains all the services and even things like the LangageServer type so they can send messages back to the client. They cant add anything to the provider, but at least they can get all the goodness if they want to dive really deep.

In this refactor I also thought... hmmm... let's go binary module this time since it probably should have been all along. So the markdown docs and maml are included.

FEEDBACK (aka commit 3+)

SeeminglyScience

Awesome stuff man :) Here's my first pass

Also:

They cant add anything to the provider, but at least they can get all the goodness if they want to dive really deep.

How feasible is that to add? Not suggesting it be done in this PR or maybe even before release, just curious what kind of a lift it is.

src/PowerShellEditorServices.Engine/Services/PowerShellContext/Extensions/EditorObject.cs

src/PowerShellEditorServices.VSCode/Cmdlets/VSCodeHtmlContentViewCommands.cs

SeeminglyScience · 2019-09-30T11:47:02Z

src/PowerShellEditorServices.VSCode/Cmdlets/VSCodeHtmlContentViewCommands.cs

+        public ViewColumn? ShowInColumn { get; set; }
+
+        ///
+        protected override void BeginProcessing()


Remove empty override

fixed all of these.

src/PowerShellEditorServices.VSCode/Cmdlets/VSCodeHtmlContentViewCommands.cs

src/PowerShellEditorServices.VSCode/CustomViews/CustomViewFeature.cs

src/PowerShellEditorServices.VSCode/CustomViews/CustomViewMessages.cs

...owerShellEditorServices.VSCode/en-US/Microsoft.PowerShell.EditorServices.VSCode.dll-Help.xml

SeeminglyScience · 2019-09-30T12:39:24Z

src/PowerShellEditorServices.Engine/Hosting/BuildInfo.cs

+    {
+        public const string BuildVersion = "<development-build>";
+        public const string BuildOrigin = "<development>";
+        public static readonly System.DateTime? BuildTime = System.DateTime.Parse("2019-09-29T23:13:58");


Why nullable? If Parse fails it'll throw during type initialization.

@rjmholt do you wanna speak to this?

I actually don't think I was suppose to checkin this file

I need to go back and fix the way we handle that file in general, but I think it's nullable so it can be built with null.

I'm not very attached to this idea; it was supposed to be a way to stamp a build to be self-documenting, but I don't think it's ever quite worked, possibly because of git checkin issues.

We can ignore it for now and I'll try and look into it in the coming weeks.

Yeah looks like we should just delete this

src/PowerShellEditorServices.VSCode/Cmdlets/VSCodeHtmlContentViewCommands.cs

TylerLeonhardt · 2019-09-30T14:40:41Z

@SeeminglyScience this is the review that was needed long long ago lol

Co-Authored-By: Patrick Meinecke <SeeminglyScience@users.noreply.github.com>

TylerLeonhardt · 2019-09-30T16:01:02Z

How feasible is that to add? Not suggesting it be done in this PR or maybe even before release, just curious what kind of a lift it is.

I mean, I guess it's feasible to bring back the ComponentRegistry and then selectively put services inside of it to expose to users... But I kinda like this model of "here's what PSES gives you, maintain your own state if you need to."

SeeminglyScience · 2019-09-30T17:03:45Z

How feasible is that to add? Not suggesting it be done in this PR or maybe even before release, just curious what kind of a lift it is.

I mean, I guess it's feasible to bring back the ComponentRegistry and then selectively put services inside of it to expose to users... But I kinda like this model of "here's what PSES gives you, maintain your own state if you need to."

Oh I took that to mean "you can't register any new things like code lens or symbol providers". All I'm worried about is if you can extend, and if other users can invoke your extensions.

e.g. with symbol providers, you can register a symbol provider, and a consumer of the PSES API can get your symbols through without referencing your project. And just to clarify, it's not a show stopper if it's not currently possible (since practically no one has used the existing system). I'd just like it to be kept in mind so that it could be re-added in the future (assuming it isn't already possible to do what I'm describing).

TylerLeonhardt · 2019-09-30T20:27:21Z

Yes I'm pretty sure that's still possible because those providers are within some service that can be accessed via the IServiceRegistry.

I'll double check but I'm pretty sure what you're asking is still possible.

rjmholt

Is this the point at which Engine should drop out of the namespace?

rjmholt · 2019-09-30T23:42:01Z

src/PowerShellEditorServices.Engine/Hosting/BuildInfo.cs

+    {
+        public const string BuildVersion = "<development-build>";
+        public const string BuildOrigin = "<development>";
+        public static readonly System.DateTime? BuildTime = System.DateTime.Parse("2019-09-29T23:13:58");


Yeah looks like we should just delete this

rjmholt · 2019-09-30T23:51:51Z

src/PowerShellEditorServices.VSCode/Cmdlets/VSCodeHtmlContentViewCommands.cs

@@ -53,25 +54,27 @@ protected override void ProcessRecord()
                }
                else
                {
-                    throw new Exception("unable to fetch psEditor value");
+                    WriteError(


Currently this is a non-terminating error.

Would it be better as a terminating error, since not being able to find the PSEditor object we won't be able to proceed meaningfully?

Terminating here wouldn't kill a script, just a pipeline, so it would operate with the desired effect I think.

ThrowTerminatingError( new ErrorRecord( new ItemNotFoundException("Cannot find the '$psEditor' variable"), "PSEditorNotFound", ErrorCategory.ObjectNotFound, targetObject: null)); return;

I think that makes sense. Updated!

It doesn't kill the script, but in general I prefer non-terminating because that lets the user easily handle errors with the -ErrorAction common parameter. With a terminating error, you have to try/catch, so I try to leave that for rare catastrophic errors.

I don't feel particularly strongly about not using terminating in this particular case though, just adding some more information.

rjmholt · 2019-09-30T23:53:24Z

src/PowerShellEditorServices.VSCode/Cmdlets/VSCodeHtmlContentViewCommands.cs

+                        ErrorCategory.CloseError,
+                        targetObject: null));
+
+                return;


The return here looks like it might be unneeded

rjmholt · 2019-09-30T23:54:09Z

src/PowerShellEditorServices.VSCode/Cmdlets/VSCodeHtmlContentViewCommands.cs

+                        ErrorCategory.OpenError,
+                        targetObject: null));
+
+                return;


Unneeded return?

rjmholt · 2019-09-30T23:54:22Z

src/PowerShellEditorServices.VSCode/Cmdlets/VSCodeHtmlContentViewCommands.cs

+                        ErrorCategory.WriteError,
+                        targetObject: null));
+
+                return;


Unneeded return?

TylerLeonhardt · 2019-10-01T00:24:09Z

Is this the point at which Engine should drop out of the namespace?

Next PR

TylerLeonhardt · 2019-10-01T00:39:10Z

Ok I've addressed all of @SeeminglyScience and @rjmholt's feedback!

TylerLeonhardt · 2019-10-01T01:28:09Z

@rjmholt RE: removing "Engine" everywhere... you're gonna want this in another PR lol

rjmholt · 2019-10-01T17:07:20Z

you're gonna want this in another PR lol

I figured. Just wanted to make sure :)

SeeminglyScience

LGTM!

… again (PowerShell#1046) * delete other folders and tweak build script for BuildInfo * working PowerShellEditorServices.VSCode now a binary module! * some typo * Apply suggestions from code review Co-Authored-By: Patrick Meinecke <SeeminglyScience@users.noreply.github.com> * address additional comments * don't checkin maml * add error handling * deleted buildinfo and address rob's comments

* Add starting point * x * More work * Make integration tests pass for omnisharp * Changes * add dummy workspace symbols handler * use LoggerFactory * A working WorkspaceSymbolsHandler * working text document syncer * needed document selector and getVersion handler to work with vscode * Added Diagnostics * didChangeConfiguration message and general settings support * Add diagnostics (#18) * initial folding support * added test for folding * Add setting support (#19) * Added Diagnostics * didChangeConfiguration message and general settings support * Apply suggestions from code review Co-Authored-By: Robert Holt <rjmholt@gmail.com> * Folding support (#20) * Added Diagnostics * didChangeConfiguration message and general settings support * initial folding support * log level trace * folding works with latest omnisharp version * comment typo * added test for folding * formatting support * remove merge conflict * add formatting tests * DocumentSymbols and References support (#997) * working formatting * add tests * delete commented out code * [Omnisharp-LSP] textDocument/documentHighlight support (#999) * Add handler scaffold * More stuff * Make handler work * Add copyright * Add tests, fix bugs * Fix small issues * codelens support (#1001) * codelens support * address rob's feedback * powerShell/getPSHostProcesses and powerShell/getRunspace (#1002) * Test only pester for now (#1003) * Implement textDocument/codeAction (#1004) * Add initial handler * Add working codeAction implementation * Crash * Make tests work * Fix issues * Make tests work in WinPS * Add powershellcontext (#1005) * Add powershellcontext * using file sink now instead * all the newlines * support $psEditor (#1006) * support $psEditor * deleted commented out code * fix initial build failures due to lack of certain assemblies * use different RootPath * wait an extra 5 seconds just in case * refactor initialize script * Re-add Stdio option and replace Pester tests with xunit tests. (#1008) * Completion Support (#1007) * completion support * misc codacy fixes * use BUILD_ARTIFACTSTAGINGDIRECTORY so logs can be uploaded * publish artifacts even if build fails * handle log messages * give PSES a chance to run what it needs to run * switch to using xUnit output helper * treat DynamicKeywords as Keyword * completionresolve support (#1009) * handle log messages * switch to using xUnit output helper * Add completionItem/resolve request * feedback * update build to run update-help for signature help test * removing scope hoping it works in CI * setting to EA silentlycontinue * change to language=powershell * hover support (#1010) * handle log messages * switch to using xUnit output helper * add hover handler * move to language=powershell * refactoring for feedback * codacy * Omni signaturehelp (#1011) * handle log messages * switch to using xUnit output helper * Support SignatureHelp * concurrentdict * Add definition handler (#1013) * add definition handler * codacy * sneak in powerShell/executionStatusChanged * codacy * Add Plaster messages (#1014) * Comment Help and Evaluate (#1015) * Support for Comment Help generator * add evaluate handler * Last LSP messages (#1016) * support CommandExporer commands and powerShell/runspaceChanged * expand alias * refactor server setup (#1018) * rename namespaces (#1019) * The entire Debug Adapter moved over... (#1043) * initial non-working dap * working launch but not attach * working attach handler * update namespaces * Disconnect support and handling of JsonRpcServer teardown * Add foundation for debug tests - stdio and fixures * all handlers * remote file manager working * rest of debug adapter * use actual release * Apply suggestions from code review Co-Authored-By: Robert Holt <rjmholt@gmail.com> * Delete projects we wont be keeping around and get pses.vscode working again (#1046) * delete other folders and tweak build script for BuildInfo * working PowerShellEditorServices.VSCode now a binary module! * some typo * Apply suggestions from code review Co-Authored-By: Patrick Meinecke <SeeminglyScience@users.noreply.github.com> * address additional comments * don't checkin maml * add error handling * deleted buildinfo and address rob's comments * Remove engine from files and namespaces (#1048) * apply apt state for PS7 (#1051) * delete buildinfo * implement powerShell/startDebugger (#1049) * implement powerShell/startDebugger * add line Co-Authored-By: Patrick Meinecke <SeeminglyScience@users.noreply.github.com> * Enable alias corrections (#1053) * Codacy comments

TylerLeonhardt added 2 commits September 29, 2019 11:47

delete other folders and tweak build script for BuildInfo

9095ebd

working PowerShellEditorServices.VSCode now a binary module!

cfb31ca

TylerLeonhardt requested a review from rjmholt September 30, 2019 06:23

TylerLeonhardt requested review from rkeithhill and SeeminglyScience as code owners September 30, 2019 06:23

some typo

65b0077

PowerShell deleted a comment Sep 30, 2019

SeeminglyScience reviewed Sep 30, 2019

View reviewed changes

Apply suggestions from code review

fb7245c

Co-Authored-By: Patrick Meinecke <SeeminglyScience@users.noreply.github.com>

PowerShell deleted a comment Sep 30, 2019

address additional comments

e1ba479

PowerShell deleted a comment Sep 30, 2019

don't checkin maml

b511511

PowerShell deleted a comment Sep 30, 2019

add error handling

6da984c

PowerShell deleted a comment Sep 30, 2019

rjmholt reviewed Sep 30, 2019

View reviewed changes

deleted buildinfo and address rob's comments

9e1d413

PowerShell deleted a comment Oct 1, 2019

rjmholt approved these changes Oct 1, 2019

View reviewed changes

SeeminglyScience approved these changes Oct 1, 2019

View reviewed changes

TylerLeonhardt merged commit 7d4516f into PowerShell:omnisharp-lsp Oct 1, 2019

TylerLeonhardt deleted the delete-code-pses.vscode-working branch October 1, 2019 18:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Delete projects we wont be keeping around and get pses.vscode working again #1046

Delete projects we wont be keeping around and get pses.vscode working again #1046

TylerLeonhardt commented Sep 30, 2019 •

edited

Loading

SeeminglyScience left a comment

SeeminglyScience Sep 30, 2019

TylerLeonhardt Sep 30, 2019

SeeminglyScience Sep 30, 2019

TylerLeonhardt Sep 30, 2019

TylerLeonhardt Sep 30, 2019

rjmholt Sep 30, 2019

rjmholt Sep 30, 2019

TylerLeonhardt commented Sep 30, 2019

TylerLeonhardt commented Sep 30, 2019

SeeminglyScience commented Sep 30, 2019

TylerLeonhardt commented Sep 30, 2019

rjmholt left a comment

rjmholt Sep 30, 2019

rjmholt Sep 30, 2019

TylerLeonhardt Oct 1, 2019

SeeminglyScience Oct 1, 2019

rjmholt Sep 30, 2019

TylerLeonhardt Oct 1, 2019

rjmholt Sep 30, 2019

TylerLeonhardt Oct 1, 2019

rjmholt Sep 30, 2019

TylerLeonhardt Oct 1, 2019

TylerLeonhardt commented Oct 1, 2019

TylerLeonhardt commented Oct 1, 2019

TylerLeonhardt commented Oct 1, 2019

rjmholt commented Oct 1, 2019

SeeminglyScience left a comment

Delete projects we wont be keeping around and get pses.vscode working again #1046

Delete projects we wont be keeping around and get pses.vscode working again #1046

Conversation

TylerLeonhardt commented Sep 30, 2019 • edited Loading

THE DELETIONS (aka commit 1)

PSES.VSCODE IS A BINARY MODULE (aka commit 2)

FEEDBACK (aka commit 3+)

SeeminglyScience left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TylerLeonhardt commented Sep 30, 2019

TylerLeonhardt commented Sep 30, 2019

SeeminglyScience commented Sep 30, 2019

TylerLeonhardt commented Sep 30, 2019

rjmholt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TylerLeonhardt commented Oct 1, 2019

TylerLeonhardt commented Oct 1, 2019

TylerLeonhardt commented Oct 1, 2019

rjmholt commented Oct 1, 2019

SeeminglyScience left a comment

Choose a reason for hiding this comment

TylerLeonhardt commented Sep 30, 2019 •

edited

Loading