Skip to content

Comments

Fix installation of versionned bundled with github type#12

Merged
wherka-ama merged 7 commits intoAmadeusITGroup:mainfrom
gblanc-1a:bugfix/issue_11_download_bundle
Dec 2, 2025
Merged

Fix installation of versionned bundled with github type#12
wherka-ama merged 7 commits intoAmadeusITGroup:mainfrom
gblanc-1a:bugfix/issue_11_download_bundle

Conversation

@gblanc-1a
Copy link
Contributor

@gblanc-1a gblanc-1a commented Nov 24, 2025

Description

Fix right-click context menu behavior so that the correct item is selected / acted on when invoked via right-click, aligning the context menu actions with the item under the cursor.

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📝 Documentation update
  • ♻️ Code refactoring (no functional changes)
  • ⚡ Performance improvement
  • 🧪 Test coverage improvement
  • 🔧 Configuration/build changes

Related Issues

Closes #
Fixes #
Relates to #

Changes Made

  • Ensure right-click events correctly set the active/selected item before opening the context menu.
  • Guard context menu actions so they operate on the item associated with the right-click target instead of any previously selected item.
  • Adjust event handling to avoid unintended selection changes when interacting with the context menu.

Testing

Test Coverage

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing completed
  • All existing tests pass

Manual Testing Steps

  1. Open the registry view and locate a list/tree item with a context menu (e.g., a prompt/entry).
  2. Right-click on several different items in succession and open the context menu each time.
  3. Trigger actions from the context menu (e.g., open/edit/delete/etc.) and verify that each action targets the item that was right-clicked.

Tested On

  • macOS

  • Windows

  • Linux

  • VS Code Stable

  • VS Code Insiders

Screenshots

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings or errors
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published

Documentation

  • README.md updated
  • JSDoc comments added/updated
  • No documentation changes needed

Additional Notes

  • This change is scoped specifically to right-click / context menu behavior and should not affect left-click selection or keyboard navigation.

Reviewer Guidelines

Please pay special attention to:

  • Event handling for right-click vs. left-click to ensure no regressions in normal selection behavior.
  • That all context menu actions consistently operate on the correct, right-clicked item across different list/tree views.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache License 2.0.

@wherka-ama
Copy link
Member

@gblanc-1a : we're very close here. It only needs npm audit fix and commiting the package-lock.json.
TIA!

@gblanc-1a gblanc-1a force-pushed the bugfix/issue_11_download_bundle branch 5 times, most recently from b5e937f to b8f5ebe Compare December 1, 2025 13:18
@gblanc-1a gblanc-1a marked this pull request as ready for review December 1, 2025 13:28
@gblanc-1a gblanc-1a force-pushed the bugfix/issue_11_download_bundle branch from b8f5ebe to abcb40a Compare December 1, 2025 15:49

if (token) {
headers['Authorization'] = `Bearer ${token}`;
headers['Authorization'] = `token ${token}`;
Copy link
Member

Choose a reason for hiding this comment

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

Was it an intentional change? Have we tested it on private repos?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes this change was intentionnal because it is indeed the legacy way of doing but it allow to support classic github token. Which are not supported by the Bearer token.

If you think we should keep Bearer we can

Copy link
Member

Choose a reason for hiding this comment

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

No need. If it works with the new scheme I'm fully aligned with that.

// },
{
label: '$(github) GitHub Releases',
description: 'Versionned releases with zip file in the assets for both public or private GitHub repository',
Copy link
Member

Choose a reason for hiding this comment

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

A little typo - versioned.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

const token = this.getAuthToken();
if (token && this.requiresAuthentication()) {
headers['Authorization'] = `Bearer ${token}`;
headers['Authorization'] = `token ${token}`;
Copy link
Member

Choose a reason for hiding this comment

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

Same remark as before regarding the change of the scheme.

@gblanc-1a gblanc-1a force-pushed the bugfix/issue_11_download_bundle branch from d22f6ed to 0a38393 Compare December 2, 2025 12:48
Copy link
Member

@wherka-ama wherka-ama left a comment

Choose a reason for hiding this comment

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

It is pretty hard to review such a large PR, but we knew it would happen and at this stage that's quite normal.

Let's assume we're not breaking it too much ;-)

Thanks a lot for your contribution @gblanc-1a ❤️

@wherka-ama
Copy link
Member

@gblanc-1a : do you mind resolving the conflicts on README or you'd ratherif I do it?

@gblanc-1a gblanc-1a force-pushed the bugfix/issue_11_download_bundle branch 2 times, most recently from 59849e7 to a151206 Compare December 2, 2025 15:52
@gblanc-1a gblanc-1a force-pushed the bugfix/issue_11_download_bundle branch from a151206 to 0fdf86e Compare December 2, 2025 15:52
@wherka-ama wherka-ama merged commit df5bb31 into AmadeusITGroup:main Dec 2, 2025
12 checks passed
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.

2 participants