Skip to content
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

getActiveInteractiveWindow to getActiveOrAssociatedInteractiveWindow #10160

Conversation

rebornix
Copy link
Member

Follow up of #10153

As @DonJayamanne pointed out in #10153 (comment), IInteractiveWindowProvider already has activeWindow getter, which gives you the currently active/focused interactive window instance in VS Code. The newly introduced getActiveInteractiveWindow is ambiguous as it actually does two things:

  • get active interactive window, same as activeWindow getter
  • if there is no activeWindow but there is an active text editor, find its associated interactive window

Thus I think it's better to rename this function to getActiveOrAssociatedInteractiveWindow to reflect its purpose better.

  • [x Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR).
  • Title summarizes what is changing.
  • Has a news entry file (remember to thank yourself!).
  • Appropriate comments and documentation strings in the code.
  • Has sufficient logging.
  • Has telemetry for feature-requests.
  • Unit tests & system/integration tests are added/updated.
  • Test plan is updated as appropriate.
  • package-lock.json has been regenerated by running npm install (if dependencies have changed).

@rebornix rebornix requested a review from a team as a code owner May 27, 2022 17:09
@rebornix rebornix self-assigned this May 27, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #10160 (d124a80) into main (e2ccb63) will increase coverage by 0%.
The diff coverage is 0%.

@@          Coverage Diff          @@
##           main   #10160   +/-   ##
=====================================
  Coverage    61%      61%           
=====================================
  Files       203      203           
  Lines      9237     9240    +3     
  Branches   1491     1492    +1     
=====================================
+ Hits       5638     5640    +2     
+ Misses     3125     3124    -1     
- Partials    474      476    +2     
Impacted Files Coverage Δ
src/platform/common/activeEditorContext.ts 93% <0%> (ø)
src/platform/errors/jupyterInvalidKernelError.ts 66% <0%> (-34%) ⬇️
src/platform/common/net/httpClient.ts 55% <0%> (-5%) ⬇️
src/platform/errors/types.ts 41% <0%> (-4%) ⬇️
src/platform/common/cancellation.ts 59% <0%> (-2%) ⬇️
src/platform/errors/errorHandler.ts 54% <0%> (+2%) ⬆️

@rebornix rebornix added this to the June 2022 milestone Jun 2, 2022
@rebornix rebornix merged commit 453f25d into microsoft:main Jun 3, 2022
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.

5 participants