Propagate and handle connection/disconnection events #1142

sebjulliand · 2023-03-16T22:28:59Z

Changes

This PR follows changes introduced by #1110.
Disconnecting and connecting to another IBM i would leave the extension in a state where some part would remain initialized for the previous connection:

Debug was not checked again
CL prompting tool was not checked again
Last connection setting was not updated
The member file system provider remained configured for the previous connection's settings

The PR mainly reorganize the extension initialisation process and leverage the connected / disconnected events to handle connection changes.

Noticeable changes:

Available events are now enumerated using the IBMiEvent type (makes it easier to use and find event references)
member file system provider now dynamically adapts to the current connection's settings and its changes. Most of the related code has been reorganized and converted to TypeScript
Some extension's subscriptions have been moved from instantiate.ts to extension.ts
Some initialization methods have been split in two: actual initialization and on connected event intialization; mainly for Debug, Deployment, Instantiate and QSysFS
Every command requiring a connection is now only visible in the command palette when connected

Checklist

have tested my change
eslint is not complaining

chrjorgensen · 2023-03-17T07:44:40Z

@sebjulliand I cannot comment on the code, but a quick run showed the following:

A click on the force reconnect button in the bar does not reconnect to same system, but goes to list of previous connections. Is that intended?
You took out Enable Source Dates from the restart group, but changing the settings does not change the behavior of the connection. After disabling and enabling, the source dates are not shown. Even after a complete VS Code restart there are no source dates - and I can't make them appear again at all in any system, although the source dates settings are enabled.
I opened a filter in Object Browser and source files appeared, restarted VS Code fully and reconnected to same system - and now no source files appear before I refresh the Object Browser:

This also happens when I do a complete VS Code restart: First click on a filter in Object Browser opens the filter but shows no filter content.
When disconnecting from a system and connecting to another system, the Object Browser shows the content from the previous system, and only after a while is the filters from the current system shown.

Enough for now... 😉

Wrongly used methods reference instead of instance's

sebjulliand · 2023-03-18T15:19:20Z

Superb feedback @chrjorgensen , thanks!
I think I got it all sorted out.

A click on the force reconnect button in the bar does not reconnect to same system, but goes to list of previous connections. Is that intended?

More or less! When I worked on the Connect to previous PR, I changed the way commands worked, and the one bound to this button showed the list of system ordered by date of connection.
Now, since we can disconnect gracefully, I thought that button served no purpose anymore. So I removed the setting to show/hide the disconnect/reconnect buttons, removed the reconnect button and showed the disconnect button permanently.

You took out Enable Source Dates from the restart group, but changing the settings does not change the behavior of the connection. After disabling and enabling, the source dates are not shown. Even after a complete VS Code restart there are no source dates - and I can't make them appear again at all in any system, although the source dates settings are enabled.

My bad...I called references to methods instead of the date handler instance's methods, which resulted in a crash (your console should have been full of those). Now it's ok, instance methods get called, the date shows and if the date settings is changed with opened member, a warning is issued to ask to close the editors to apply the change.

I opened a filter in Object Browser and source files appeared, restarted VS Code fully and reconnected to same system - and now no source files appear before I refresh the Object Browser:

This also happens when I do a complete VS Code restart: First click on a filter in Object Browser opens the filter but shows no filter content.

This one I'm not sure about. I reorganized the call to the views' refresh when connecting, hopefully it should fix the issue. The views will also refresh synchronously on disconnect to empty them. There should not be any remaining items from a previous connection this way.

When disconnecting from a system and connecting to another system, the Object Browser shows the content from the previous system, and only after a while is the filters from the current system shown.

Fixed!

Enough for now... 😉

Right! Enjoy your week-end 😉

worksofliam · 2023-03-19T02:29:01Z

Woah, big change indeed. I've had a review and I appreciate your notes about the significant changes. The biggest change here that I am worried is to the member file system, in particular continuing to support 'edit' and 'diff' mode when source dates are enabled.

I need to sit down for a few hours and just use Code for IBM i on this branch to give it some real testing - and that's exactly what I'm going to do tomorrow (Sunday) morning.

Editing with source dates disabled
Editing with dates enabled in edit mode
Editing with dates enabled in diff mode
Compiling source code in each of those modes
Ensure that deploy continues to work
Ensure CL installation works on first connect and subsequent connections
Ensure debugging is checked on each system

@sebjulliand @chrjorgensen Seeing as this is the largest PR at the moment, would you say this should go to the top of the queue with highest priority?

Great work. I admire your skill!

sebjulliand · 2023-03-19T11:57:47Z

As always, thanks for your appreciation, it means a lot 😊
The FS change change is indeed the most important here - hopefully nothing got lost during the refactoring.

AFAIC, I would say this PR should be merged as soon as it's approved, given the amount of changes in there and the fact it fixes/enhance the "Disconnect gracefully" PR that's already been merged.

worksofliam · 2023-03-19T15:51:43Z

Ok, after my testing this is what I've found:

Compiling works as expected in all edit modes as expected
CL program gets installed on every reconnect as expected

Here's a couple of issues:

Deploy

Deploy status bar item was not showing up on the first connection even with a local folder open:

Though if I disconnected and connected to the same system again, it was shown:

Source dates

On first time connect, it was mostly working. The logic to track edits looks like it was working correctly - this is the important part!

When opening a file though, the gutter was not appearing even when I had source dates enabled (usually it has to render when the file has been opened also).

Screen.Recording.2023-03-19.at.11.42.14.AM.mov

Lastly, the biggest issue was that the gutter was not showing at all if I disconnected and reconnected to the same system. Couldn't really understand why.

worksofliam

Please see my review above.

sebjulliand · 2023-03-21T19:57:10Z

@worksofliam I fixed it all (well...at least what you found!).
The deploy action bar now shows correctly and source gutter is displayed correctly.

worksofliam · 2023-03-21T20:58:38Z

@sebjulliand i will play some more tonight! Great work!

worksofliam

Just gave your changes a whirl. I am very happy with this change. I am just have a stroll through the code and take a look and how you managed it!

@chjorgensen Would you also mind giving this another go? I think we're getting close to merging this in.

worksofliam · 2023-03-22T01:12:04Z

src/api/Instance.ts

-    getConnection() {
-      return this.connection;
-    }
+  private connection: IBMi | undefined;


This is welcome change. Fingers crossed anyone using our extension API isn't using these!

worksofliam

I am happy with my testing and the code - how grateful I am. This will allow for a much improved switch between systems. A welcome UX change.

@chrjorgensen Before we merge, I'd like your overview also. If you agree with me that it runs stable, I think we can go ahead and merge.

chrjorgensen

@sebjulliand A quick review found the following:

Disconnecting with a branch open in the object browser gives the following message:
When disconnecting, the message Disconnecting from... stays on screen. Should be Disconnected from....

It's great to be able to change the source date settings and just reopen the editor and not having to disconnect + reconnect! 👌

sebjulliand · 2023-03-22T19:06:32Z

@chrjorgensen thanks for the testing!

Disconnecting with a branch open in the object browser gives the following message:

Fixed...

When disconnecting, the message Disconnecting from... stays on screen. Should be Disconnected from....

...and fixed!

chrjorgensen

@sebjulliand LGTM - no further problems found.

@worksofliam I'll let it be up to you to approve - I have not found any more issues.

worksofliam

I am happy too. Let's merge! This will be released tonight in 1.7.6.

sebjulliand added 4 commits March 15, 2023 21:33

Only show connection dependent command when connected

ec59980

Handle disconnection propagation

a9ff448

Update index.js

5c39887

Missing handle on handler's edit timeout

26e1dfa

sebjulliand requested review from worksofliam and chrjorgensen March 16, 2023 22:29

chrjorgensen assigned worksofliam Mar 16, 2023

sebjulliand added 6 commits March 18, 2023 11:14

Fixed source dates not showing up

f224e9b

Wrongly used methods reference instead of instance's

Handle source date settings change for active editors

46b21ae

Moved "set connected" context value to IBMi

ca82003

Load views on connected event

9d6dd25

Removed force reconnect and always show disconnect

f32c80d

Force synch refresh of views on disconnect to empty them

61ecc51

worksofliam requested changes Mar 19, 2023

View reviewed changes

sebjulliand and others added 2 commits March 20, 2023 15:10

Don't let Instance share its toys

ad336f6

Fixed source dates not showing when opened/changed

2cc03b5

worksofliam reviewed Mar 22, 2023

View reviewed changes

worksofliam previously approved these changes Mar 22, 2023

View reviewed changes

chrjorgensen requested changes Mar 22, 2023

View reviewed changes

sebjulliand added 2 commits March 22, 2023 20:04

Moved and updated disconnection message

717d0e7

Clear instance's connection on disconnection

a03edd8

sebjulliand dismissed worksofliam’s stale review via a03edd8 March 22, 2023 19:05

worksofliam requested review from worksofliam and chrjorgensen March 22, 2023 19:21

chrjorgensen reviewed Mar 22, 2023

View reviewed changes

This comment was marked as duplicate.

Sign in to view

worksofliam approved these changes Mar 22, 2023

View reviewed changes

worksofliam merged commit 1df788a into master Mar 22, 2023

sebjulliand deleted the feature/disconnectionPropagation branch March 23, 2023 08:04

sebjulliand mentioned this pull request Mar 27, 2023

Added capability to sort IFS files and Members by name or date #1151

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Propagate and handle connection/disconnection events #1142

Propagate and handle connection/disconnection events #1142

sebjulliand commented Mar 16, 2023

chrjorgensen commented Mar 17, 2023

sebjulliand commented Mar 18, 2023 •

edited

Loading

worksofliam commented Mar 19, 2023

sebjulliand commented Mar 19, 2023

worksofliam commented Mar 19, 2023

worksofliam left a comment

sebjulliand commented Mar 21, 2023

worksofliam commented Mar 21, 2023

worksofliam left a comment

worksofliam Mar 22, 2023

worksofliam left a comment

chrjorgensen left a comment

sebjulliand commented Mar 22, 2023

chrjorgensen left a comment

This comment was marked as duplicate.

worksofliam left a comment

Propagate and handle connection/disconnection events #1142

Propagate and handle connection/disconnection events #1142

Conversation

sebjulliand commented Mar 16, 2023

Changes

Checklist

chrjorgensen commented Mar 17, 2023

sebjulliand commented Mar 18, 2023 • edited Loading

worksofliam commented Mar 19, 2023

sebjulliand commented Mar 19, 2023

worksofliam commented Mar 19, 2023

Deploy

Source dates

worksofliam left a comment

Choose a reason for hiding this comment

sebjulliand commented Mar 21, 2023

worksofliam commented Mar 21, 2023

worksofliam left a comment

Choose a reason for hiding this comment

worksofliam Mar 22, 2023

Choose a reason for hiding this comment

worksofliam left a comment

Choose a reason for hiding this comment

chrjorgensen left a comment

Choose a reason for hiding this comment

sebjulliand commented Mar 22, 2023

chrjorgensen left a comment

Choose a reason for hiding this comment

This comment was marked as duplicate.

worksofliam left a comment

Choose a reason for hiding this comment

sebjulliand commented Mar 18, 2023 •

edited

Loading