Skip to content

Conversation

@witness-me
Copy link
Contributor

Problem

  1. The feature opens specific file types in a custom editor by default and customers don't have a way to optout while still using toolkit
  2. Currently the indentation setting used in the integration defaults to 4 spaces. If it's different in user's VSCode setting, the spacing for the string sent from the integration will not match
  3. Currently context menu lacks the command to launch WFS. It has been added before, but it looks it was accidentally added to auto-generated file and was thus removed later

Solution

  1. Adding a setting to opt out of WFS, which will close custom editor and open default editor instead. Note that I did not find a way to completely prevent custom editor panel mount - it looks it's not possible, so it will start mounting first and then (in resolveCustomTextEditor) it needs to be witched to default editor. Please advise if that's not the case and the custom editor mount can be prevented earlier (while keeping file format association with the file types)
  2. Passing a user setting for tab spacing to be used on the webview side to format JSON/YAML with the right indentation
  3. Adding an option to launch WFS from context menu, as it was added before (but this time in the right package.json)

  • Treat all work as PUBLIC. Private feature/x branches will not be squash-merged at release time.
  • Your code changes must meet the guidelines in CONTRIBUTING.md.
  • License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

@witness-me witness-me requested a review from a team as a code owner February 4, 2025 22:37
@github-actions
Copy link

github-actions bot commented Feb 4, 2025

  • This pull request modifies code in src/* but no tests were added/updated.
    • Confirm whether tests should be added or ensure the PR description explains why tests are not required.
  • This pull request implements a feat or fix, so it must include a changelog entry (unless the fix is for an unreleased feature). Review the changelog guidelines.
    • Note: beta or "experiment" features that have active users should announce fixes in the changelog.
    • If this is not a feature or fix, use an appropriate type from the title guidelines. For example, telemetry-only changes should use the telemetry type.

@witness-me witness-me force-pushed the add-opt-out branch 3 times, most recently from c43a61a to 4b7638e Compare February 6, 2025 17:37
"default": true,
"description": "%AWS.stepFunctions.asl.format.enable.desc%"
},
"aws.stepfunctions.workflowStudio.enable": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this setting being added?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is explained in point 1:

The feature opens specific file types in a custom editor by default and customers don't have a way to optout while still using toolkit

Our team (including the UX) decided that since custom editor is opened by default, we want to provide a way for the user to opt out of it

Copy link
Contributor

@justinmk3 justinmk3 left a comment

Choose a reason for hiding this comment

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

Why is a setting being added? Not explained in the PR description. Looks like a low-value setting. Settings should not be added unless there is a strong use-case.

"AWS.stepFunctions.workflowStudio.actions.progressMessage": "Opening asl file in Workflow Studio",
"AWS.stepFunctions.workflowStudio.actions.saveSuccessMessage": "{0} has been saved",
"AWS.stepFunctions.workflowStudio.actions.invalidJson": "The Workflow Studio editor was not opened because the JSON in the file is invalid. To access Workflow Studio, please fix the JSON and manually reopen the integration.",
"AWS.stepFunctions.workflowStudio.enable.desc": "Open Amazon States Language files in Workflow Studio by default.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Use workbench.editorAssociations. We already do something similar for threat composer:

"configurationDefaults": {
"workbench.editorAssociations": {
"{git,gitlens,conflictResolution,vscode-local-history}:/**/*.tc.json": "default"
}

Do not add new, custom ways of setting file associations.

@witness-me
Copy link
Contributor Author

After discussing offline, we agreed to not add that as a setting, as this is something that can be done with local config file. I'll cancel this PR since that change was its main content, and will publish other small changes in a separate PR

@witness-me witness-me closed this Feb 6, 2025
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.

4 participants