feat: Introduce integrated terminal #3079

zFernand0 · 2024-08-29T13:51:52Z

Proposed changes

Introduces the MVS, TSO, and Unix command handlers as integrated terminals

Release Notes

Milestone: 3.1.0

Changelog: Introduces the MVS, TSO, and Unix command handlers as integrated terminals

Types of changes

Bug fix (non-breaking change which fixes an issue)
Enhancement (non-breaking change which adds or improves functionality)
Breaking change (a change that would cause existing functionality to not work as expected)
Documentation (Markdown, README updates)
Other (please specify above in "Proposed changes" section)

Checklist

General

I have read the CONTRIBUTOR GUIDANCE wiki
All PR dependencies have been merged and published (if applicable)
A GIF or screenshot is included in the PR for visual changes
The pre-publish command has been executed:
- v2 and below: yarn workspace vscode-extension-for-zowe vscode:prepublish
- v3: pnpm --filter vscode-extension-for-zowe vscode:prepublish

Code coverage

There is coverage for the code that I have added
I have added new test cases and they are passing
I have manually tested the changes

Deployment

I have added developer documentation (if applicable)
Documentation should be added to Zowe Docs
- If you're an outside contributor, please post in the #zowe-doc Slack channel to coordinate documentation.
- Otherwise, please check with the rest of the squad about any needed documentation before merging.
These changes may need ported to the appropriate branches (list here):

Further comments

Merged and published:

feat: added method to validate the ssh connection zowe-cli#2340

codecov · 2024-08-29T13:55:42Z

Codecov Report

Attention: Patch coverage is 93.38677% with 33 lines in your changes missing coverage. Please review.

Project coverage is 93.23%. Comparing base (aea6d92) to head (c998f8d).

Files with missing lines	Patch %	Lines
...s/zowe-explorer/src/commands/UnixCommandHandler.ts	88.23%	10 Missing ⚠️
...es/zowe-explorer/src/commands/TsoCommandHandler.ts	78.37%	8 Missing ⚠️
...es/zowe-explorer/src/commands/MvsCommandHandler.ts	84.37%	5 Missing ⚠️
.../zowe-explorer/src/commands/ZoweCommandProvider.ts	96.55%	4 Missing ⚠️
...-explorer-api/src/profiles/ZoweExplorerZosmfApi.ts	62.50%	3 Missing ⚠️
packages/zowe-explorer/src/tools/ZoweTerminal.ts	98.95%	2 Missing ⚠️
...ckages/zowe-explorer/src/configuration/Profiles.ts	85.71%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3079      +/-   ##
==========================================
+ Coverage   93.17%   93.23%   +0.05%     
==========================================
  Files         117      118       +1     
  Lines       12277    12393     +116     
  Branches     2822     2856      +34     
==========================================
+ Hits        11439    11554     +115     
- Misses        837      838       +1     
  Partials        1        1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

sonarqubecloud · 2024-09-16T20:01:53Z

Quality Gate failed

Failed conditions
22.7% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

…terminals 😋 Signed-off-by: zFernand0 <[email protected]>

Signed-off-by: zFernand0 <[email protected]>

packages/zowe-explorer/src/commands/UnixCommandHandler.ts

packages/zowe-explorer/src/webviews/src/edit-history/App.tsx

Signed-off-by: zFernand0 <[email protected]>

Signed-off-by: Fernando Rijo Cedeno <[email protected]>

…nto feat/int-term Signed-off-by: Fernando Rijo Cedeno <[email protected]>

Signed-off-by: Fernando Rijo Cedeno <[email protected]>

packages/zowe-explorer/__tests__/__unit__/tools/ZoweTerminal.unit.test.ts

Signed-off-by: Fernando Rijo Cedeno <[email protected]>

packages/zowe-explorer/__tests__/__unit__/commands/ZoweCommandProvider.unit.test.ts

Signed-off-by: Fernando Rijo Cedeno <[email protected]>

zFernand0

Addressing some comments:

All input is blocked when a command is running (except for ctrl+C to cancel the command)

package.json

packages/zowe-explorer-api/src/profiles/UserSettings.ts

packages/zowe-explorer/src/commands/ZoweCommandProvider.ts

…nto feat/int-term Signed-off-by: Fernando Rijo Cedeno <[email protected]>

…ahracters Signed-off-by: Fernando Rijo Cedeno <[email protected]>

…nto feat/int-term Signed-off-by: Fernando Rijo Cedeno <[email protected]>

Changes have been addressed 🙏

Signed-off-by: Fernando Rijo Cedeno <[email protected]>

packages/zowe-explorer/src/tools/ZoweTerminal.ts

Signed-off-by: Fernando Rijo Cedeno <[email protected]>

adam-wolfe · 2024-12-11T22:10:44Z

Something after b707bdb seems to have broken my ability to launch an integrated terminal. Instead, it takes me to the quick pick for entering a TSO command.

zFernand0 · 2024-12-12T15:48:16Z

Something after b707bdb seems to have broken my ability to launch an integrated terminal. Instead, it takes me to the quick pick for entering a TSO command.

The default setting for the value was changed to false, making this an opt-in feature that can be released at any time 🙏

adam-wolfe

Looks great, thanks @zFernand0. Will re-review/approve when out of draft.

traeok

Thanks for addressing all the feedback Fernando 😋 I had a couple questions regarding behaviors that I noticed while using the terminal, but happy to approve once the PR is ready to go 😁

Environment variables don't seem to work in the context of Unix/SSH. We might want to warn users that environment variables cannot be set when issuing Unix commands as part of the integrated terminal doc. Do we want to handle this as a separate enhancement? Thanks for clarifying on the environment variables, I removed my examples because the first one should technically work with && between the two statements (env. var and the echo), and the second is improper syntax 😅
A few keys render as "replacement characters," sometimes with additional escape sequences afterward. Do we want to ignore these inputs or keep this behavior as-is? I tried the following sequences (in order of characters in screenshot):
- Tab
- Ctrl+Backspace
- Page Up (rendered as �[5~)
- Page Down (rendered as �[6~)
- Ctrl+Left Arrow (shown twice, rendered as �[1;5D)
- Ctrl+Right Arrow (rendered as �[1;5C)

Signed-off-by: Fernando Rijo Cedeno <[email protected]>

zFernand0 · 2024-12-18T18:56:25Z

Thanks @adam-wolfe and @traeok for testing.
I've just added a few more edge cases (with modifier keys) and made the "exit" approach the same as the node shell/interpreter 🙏
(To exit, press Ctrl+C again or Ctrl+D or type .exit) 😋

Trae, the ENV vars is a good catch.
As long as you split it in separate commands, it should work just fine.
Also, to resolve ENV vars, I believe echo requires "
MY_CUSTOM_VAR=world; echo "Hello $MY_CUSTOM_VAR"

Signed-off-by: Fernando Rijo Cedeno <[email protected]>

t1m0thyj · 2025-01-03T21:48:05Z

packages/zowe-explorer-api/src/extend/MainframeInteraction.ts

+         * @returns {Promise<zostso.IIssueResponse>}
+         * @memberof ICommand
+         */
+        issueTsoCmdWithParms?(command: string, parms?: zostso.IStartTsoParms): Promise<zostso.IIssueResponse>;


Since the old and new method have the same function signature, curious whether we prefer this approach of adding a new method, or adding an optional boolean param to the existing method to change its behavior?

t1m0thyj · 2025-01-03T21:49:10Z

packages/zowe-explorer-api/src/tree/ZoweTreeNode.ts

     */
-    public setProfileToChoice(aProfile: imperative.IProfileLoaded, fsProvider?: BaseProvider): void {
+    public setProfileToChoice(aProfile: imperative.IProfileLoaded): void {


Is this a breaking change in ZE API? Or is it ok since the param was optional/unused?

t1m0thyj · 2025-01-03T21:50:27Z

packages/zowe-explorer/__tests__/__unit__/commands/UnixCommandHandler.unit.test.ts

                zosmfProfile: mockLoadNamedProfile,
                checkCurrentProfile: jest.fn(() => {
                    return profilesForValidation;
                }),
+                profileValidationHelper: jest.fn().mockReturnValue("active"), //.mockImplementation((prof, fun) => fun(prof)),


Can the comment be removed?

t1m0thyj · 2025-01-03T21:53:38Z

packages/zowe-explorer/package.nls.json

+  "zowe.commands.alwaysEdit": "Allow editing of commands before submitting",
+  "zowe.commands.useIntegratedTerminals": "Allow commands to be executed using integrated terminals",


Should we make the wording more consistent across settings?

Suggested change

"zowe.commands.alwaysEdit": "Allow editing of commands before submitting",

"zowe.commands.useIntegratedTerminals": "Allow commands to be executed using integrated terminals",

"zowe.commands.alwaysEdit": "Allow commands to be edited before submitting",

"zowe.commands.useIntegratedTerminals": "Allow commands to be executed using integrated terminals",

t1m0thyj · 2025-01-03T21:55:39Z

packages/zowe-explorer/src/commands/MvsCommandHandler.ts

@@ -42,13 +40,32 @@ export class MvsCommandHandler extends ZoweCommandProvider {
        return this.instance;
    }

-    private static readonly defaultDialogText: string = `$(plus) ${vscode.l10n.t("Create a new MVS command")}`;
+    public readonly controller: AbortController = new AbortController();


Could the controller variable definition be moved to a common place in ZoweCommandProvider? Not sure if that makes sense but I noticed it seems to be duplicated for each command type 😋

pull-request-size bot added the size/XXL label Aug 29, 2024

zFernand0 self-assigned this Aug 29, 2024

adam-wolfe added this to the v3.1.0 milestone Sep 3, 2024

zFernand0 mentioned this pull request Sep 5, 2024

Port zosconsole to next #3058

Merged

15 tasks

zFernand0 added 8 commits October 9, 2024 09:41

chore: remove code duplication in preparation to handling integrated …

0f14050

…terminals 😋 Signed-off-by: zFernand0 <[email protected]>

chore: done consolidating, fixing unit test before continuing 😋

73f5814

Signed-off-by: zFernand0 <[email protected]>

tests: fix unit tests and address any missing checks 😋

ee95d66

Signed-off-by: zFernand0 <[email protected]>

tests: fix the rest of the unit tests 😋

48abeef

Signed-off-by: zFernand0 <[email protected]>

l10n: update strings 😋

43d6863

Signed-off-by: zFernand0 <[email protected]>

chore: Add zoweTerminal 😋

beba0c4

Signed-off-by: zFernand0 <[email protected]>

chore: finalize ZoweTerminal implementation 😋

d3b5975

Signed-off-by: zFernand0 <[email protected]>

chore: add actual z/os behavior 😋

4f7dd21

Signed-off-by: zFernand0 <[email protected]>

zFernand0 force-pushed the feat/int-term branch from 525170c to 4f7dd21 Compare October 9, 2024 13:54

zFernand0 added 4 commits October 9, 2024 09:56

chore: add missing license header

451c49b

Signed-off-by: zFernand0 <[email protected]>

tests: fix unit tests assuming we are not using integrated terminals

978ff99

Signed-off-by: zFernand0 <[email protected]>

feat: initial addition of zowe commands to edit History webview

8b2d310

Signed-off-by: zFernand0 <[email protected]>

fix: done with core terminal issues

ee12695

Signed-off-by: zFernand0 <[email protected]>

github-advanced-security bot found potential problems Oct 9, 2024

View reviewed changes

packages/zowe-explorer/src/commands/UnixCommandHandler.ts Fixed Show fixed Hide fixed

packages/zowe-explorer/src/webviews/src/edit-history/App.tsx Fixed Show fixed Hide fixed

zFernand0 added 5 commits October 10, 2024 10:06

tests: fix unit tests for now 😋

859807c

Signed-off-by: zFernand0 <[email protected]>

fix remainig unit tests

6d49d02

Signed-off-by: Fernando Rijo Cedeno <[email protected]>

Merge branch 'main' of https://github.com/zowe/zowe-explorer-vscode i…

65c8e24

…nto feat/int-term Signed-off-by: Fernando Rijo Cedeno <[email protected]>

fix: remove circular references

04927a3

Signed-off-by: Fernando Rijo Cedeno <[email protected]>

test: add small unit test 😋

96a8d25

Signed-off-by: Fernando Rijo Cedeno <[email protected]>

github-advanced-security bot found potential problems Oct 21, 2024

View reviewed changes

packages/zowe-explorer/__tests__/__unit__/tools/ZoweTerminal.unit.test.ts Fixed Show fixed Hide fixed

packages/zowe-explorer/__tests__/__unit__/tools/ZoweTerminal.unit.test.ts Fixed Show fixed Hide fixed

test: add more unit testws to bump patch coverage

257254b

Signed-off-by: Fernando Rijo Cedeno <[email protected]>

github-advanced-security bot found potential problems Oct 22, 2024

View reviewed changes

zFernand0 added 2 commits October 22, 2024 11:52

chore: more unit tests

819e742

Signed-off-by: Fernando Rijo Cedeno <[email protected]>

Merge branch 'main' into feat/int-term

3898c54

review: address lint issues

50fd881

Signed-off-by: Fernando Rijo Cedeno <[email protected]>

JTonda marked this pull request as draft November 26, 2024 16:10

zFernand0 added 3 commits December 10, 2024 11:06

review: Address delete key, and cancel operations

f5546a6

Signed-off-by: Fernando Rijo Cedeno <[email protected]>

review: revert typecript update

5e5242f

Signed-off-by: Fernando Rijo Cedeno <[email protected]>

review: remove redundant logs

725e369

Signed-off-by: Fernando Rijo Cedeno <[email protected]>

zFernand0 commented Dec 10, 2024

View reviewed changes

zFernand0 added 3 commits December 10, 2024 12:25

Merge branch 'main' of https://github.com/zowe/zowe-explorer-vscode i…

f839d0e

…nto feat/int-term Signed-off-by: Fernando Rijo Cedeno <[email protected]>

review: fix delete key with moving cursor across emojis and special c…

09e0ce2

…ahracters Signed-off-by: Fernando Rijo Cedeno <[email protected]>

Merge branch 'main' of https://github.com/zowe/zowe-explorer-vscode i…

b707bdb

…nto feat/int-term Signed-off-by: Fernando Rijo Cedeno <[email protected]>

review: fix oversight of moving to charArray

05a892c

Signed-off-by: Fernando Rijo Cedeno <[email protected]>

github-advanced-security bot found potential problems Dec 10, 2024

View reviewed changes

packages/zowe-explorer/src/tools/ZoweTerminal.ts Fixed Show fixed Hide fixed

zFernand0 added 3 commits December 10, 2024 16:23

review: handle home and end keys

2716e21

Signed-off-by: Fernando Rijo Cedeno <[email protected]>

review: address home and end key

a9247fc

Signed-off-by: Fernando Rijo Cedeno <[email protected]>

review: display non printable characters

d58c416

Signed-off-by: Fernando Rijo Cedeno <[email protected]>

adam-wolfe previously approved these changes Dec 12, 2024

View reviewed changes

Merge branch 'main' into feat/int-term

f4d60dd

traeok self-requested a review December 18, 2024 15:03

traeok reviewed Dec 18, 2024

View reviewed changes

review: fix modifier keys and add intuitive control+C exit

4d498c4

Signed-off-by: Fernando Rijo Cedeno <[email protected]>

zFernand0 dismissed adam-wolfe’s stale review via 4d498c4 December 18, 2024 18:33

zFernand0 marked this pull request as ready for review December 18, 2024 18:36

review: bump test coverage of the zowe terminal

c998f8d

Signed-off-by: Fernando Rijo Cedeno <[email protected]>

zFernand0 force-pushed the feat/int-term branch from de4af46 to c998f8d Compare December 19, 2024 21:09

t1m0thyj linked an issue Dec 23, 2024 that may be closed by this pull request

Add support for integrated terminals (MVS, SSH, TSO) #3071

Open

t1m0thyj reviewed Jan 3, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Introduce integrated terminal #3079

feat: Introduce integrated terminal #3079

zFernand0 commented Aug 29, 2024 •

edited

Loading

codecov bot commented Aug 29, 2024 •

edited

Loading

sonarqubecloud bot commented Sep 16, 2024 •

edited

Loading

zFernand0 left a comment

adam-wolfe commented Dec 11, 2024

zFernand0 commented Dec 12, 2024

adam-wolfe left a comment •

edited

Loading

traeok left a comment •

edited

Loading

zFernand0 commented Dec 18, 2024

t1m0thyj Jan 3, 2025

t1m0thyj Jan 3, 2025

t1m0thyj Jan 3, 2025

t1m0thyj Jan 3, 2025

t1m0thyj Jan 3, 2025

		"zowe.commands.alwaysEdit": "Allow editing of commands before submitting",
		"zowe.commands.useIntegratedTerminals": "Allow commands to be executed using integrated terminals",

feat: Introduce integrated terminal #3079

Are you sure you want to change the base?

feat: Introduce integrated terminal #3079

Conversation

zFernand0 commented Aug 29, 2024 • edited Loading

Proposed changes

Release Notes

Types of changes

Checklist

Further comments

codecov bot commented Aug 29, 2024 • edited Loading

Codecov Report

sonarqubecloud bot commented Sep 16, 2024 • edited Loading

Quality Gate failed

zFernand0 left a comment

Choose a reason for hiding this comment

adam-wolfe commented Dec 11, 2024

zFernand0 commented Dec 12, 2024

adam-wolfe left a comment • edited Loading

Choose a reason for hiding this comment

traeok left a comment • edited Loading

Choose a reason for hiding this comment

zFernand0 commented Dec 18, 2024

t1m0thyj Jan 3, 2025

Choose a reason for hiding this comment

t1m0thyj Jan 3, 2025

Choose a reason for hiding this comment

t1m0thyj Jan 3, 2025

Choose a reason for hiding this comment

t1m0thyj Jan 3, 2025

Choose a reason for hiding this comment

t1m0thyj Jan 3, 2025

Choose a reason for hiding this comment

zFernand0 commented Aug 29, 2024 •

edited

Loading

codecov bot commented Aug 29, 2024 •

edited

Loading

sonarqubecloud bot commented Sep 16, 2024 •

edited

Loading

adam-wolfe left a comment •

edited

Loading

traeok left a comment •

edited

Loading