Skip to content

update notification mode to enum add saveScope change message #1087

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

Merged
merged 5 commits into from
Jan 9, 2024

Conversation

brianignacio5
Copy link
Collaborator

@brianignacio5 brianignacio5 commented Nov 27, 2023

Description

Add notification for ESP-IDF: Select where to save configuration settings command on successful update.

Modify idf.notificationSilentMode to idf.notificationMode to allow 4 modes of user notifications and output focus: All, Notifications only, output focus only or Silent. Add ESP-IDF: Select output and notification mode to select notification mode.

Fixes VSC-1225

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Steps to test this pull request

Provide a list of steps to test changes in this PR and required output

  1. Click on ESP-IDF: Select where to save configuration settings and see success notification
  2. Click on ESP-IDF: Select output and notification mode, select notification and output mode.
  3. Execute extension actions such as build, flash, idf size and other extension tasks.
  4. Observe output focus when idf.notificationMode is set to All or Output.
  5. Observe notifications when idf.notificationMode is set to All or Notification.
  • Expected behaviour:
  • Notification are shown when when idf.notificationMode is set to All or Notification.
  • Task Output is focused when when idf.notificationMode is set to All or Output.
  • `ESP-IDF: Select where to save configuration settings will show notification message when used.

Test Configuration:

  • ESP-IDF Version: 5.0
  • OS (Windows,Linux and macOS): All

Checklist

  • PR Self Reviewed
  • Applied Code formatting
  • Added Documentation
  • Added Unit Test
  • Verified on all platforms - Windows,Linux and macOS

@brianignacio5 brianignacio5 added the enhancement New feature or request label Nov 27, 2023
@brianignacio5 brianignacio5 self-assigned this Nov 27, 2023
Copy link

github-actions bot commented Nov 27, 2023

Download the artifacts for this pull request:

Copy link
Collaborator

@radurentea radurentea left a comment

Choose a reason for hiding this comment

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

Left a small request for a missing dot

Copy link
Collaborator

@radurentea radurentea left a comment

Choose a reason for hiding this comment

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

LGTM

@brianignacio5 brianignacio5 added this to the v1.7.0 milestone Dec 19, 2023
@AndriiFilippov
Copy link
Collaborator

@brianignacio5 hi !

Tested:
OS - Windows 10
ESP-IDF: v5.1.1

Click on ESP-IDF: Select where to save configuration settings -> select "Workspace Folder" -> Click on ESP-IDF: Select output and notification mode -> select "Notification" -> error:

image

@brianignacio5
Copy link
Collaborator Author

@brianignacio5 hi !

Tested: OS - Windows 10 ESP-IDF: v5.1.1

Click on ESP-IDF: Select where to save configuration settings -> select "Workspace Folder" -> Click on ESP-IDF: Select output and notification mode -> select "Notification" -> error:

image

Fixed

@AndriiFilippov
Copy link
Collaborator

@brianignacio5 hi !

able to build.

Will you separate the "Workspace" and "Workspace Folder" ?

As we discussed like "every workspace could contain many projects" , so we could set individual notification settings for each of them ?

@brianignacio5
Copy link
Collaborator Author

@brianignacio5 hi !

able to build.

Will you separate the "Workspace" and "Workspace Folder" ?

As we discussed like "every workspace could contain many projects" , so we could set individual notification settings for each of them ?

Yes that is how it is right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants