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

Fix gear action in Find with Sourcegraph dialog #7369

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mkondratek
Copy link
Contributor

@mkondratek mkondratek commented Mar 7, 2025

There was an issue with the action...

Based on PR #7368

Chain of upstream PRs as of 2025-03-07

Test plan

  1. right click in the editor
  2. Sourcegraph / Find with Sourcegraph...
  3. click the gear icon

Before:
nothing

After this PR:
setttings file opens

@mkondratek mkondratek self-assigned this Mar 7, 2025
@mkondratek mkondratek changed the title mkondratek/fix/gear-action Fix gear action in Find with Sourcegraph dialog Mar 7, 2025
Copy link
Contributor

@dominiccooney dominiccooney left a comment

Choose a reason for hiding this comment

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

Do we really need this AnAction, can't we set up the DataContext for the button itself, and the ActionEvent will pick up that context when it is created for the button?

public void actionPerformed(@NotNull AnActionEvent e) {
new OpenCodySettingsEditorAction()
.actionPerformed(
AnActionEvent.createFromDataContext(
Copy link
Contributor

Choose a reason for hiding this comment

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

This is deprecated in tip, so don't use it.

If we have to use it, put a TODO: Migrate to X after min version passes Y

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, thanks. my intellij does not show that tip but you are right [Sourcegraph link]. I switched to withDataContext that is not deprecated.

@mkondratek
Copy link
Contributor Author

mkondratek commented Mar 10, 2025

Do we really need this AnAction, can't we set up the DataContext for the button itself, and the ActionEvent will pick up that context when it is created for the button?

I was thinking about it and tried it but couldn't get it working. It seems to me that it would require more extensive refactor to make it both clean and working. I wanted to keep this fix as simple as possible.

edit:
Tried once again with something like this:

    ActionButton goToPluginSettingsButton =
        GoToPluginSettingsButtonFactory.createGoToPluginSettingsButton();
    DataContext dataContext = DataManager.getInstance().getDataContext(goToPluginSettingsButton);
    DataManager.getInstance()
        .saveInDataContext(dataContext, Key.create(CommonDataKeys.PROJECT.getName()), project);

But I cannot retrieve the project in the action. It's not included in the action event's data (as it's assigned to the component). I tried to get the component there but I could not find a way to do so and it started to look much more messy 😕

I think my present solution is simple, quick, working and it is already implemented so I'd rather keep it for now. We could rethink and refactor that some day, e.g. when we decide to move all sourcegraph related code to kotlin.

Base automatically changed from mkondratek/chore/cleanup-icons to main March 10, 2025 10:29
@mkondratek mkondratek force-pushed the mkondratek/fix/gear-action branch from 554d0fa to 4840f7a Compare March 10, 2025 13:47
mkondratek added a commit that referenced this pull request Mar 10, 2025
This was an issue for a long time. I noticed that the newly updated
sourcegraph icon appears under Sourcegraph section and compared the
svgs. Cody helped me to identify the part that is not supported.

JetBrains does not support color definition `color(display-p3 ...)`.
Each icon has a hex definition anyway. The one that is not supported is
redundant so I removed it and it works now 🚀

<!-- start git-machete generated -->

# Based on PR #7369

## Chain of upstream PRs as of 2025-03-07

* PR #7368:
  `main` ← `mkondratek/chore/cleanup-icons`

  * PR #7369:
    `mkondratek/chore/cleanup-icons` ← `mkondratek/fix/gear-action`

    * **PR #7370 (THIS ONE)**:
      `mkondratek/fix/gear-action` ← `mkondratek/fix/icon-colors`

<!-- end git-machete generated -->

## Test plan
Right click in the editor & review Cody actions - expected icons
visible.
Also, shift shift, "Cody" review the icons visible.

### Demo - before

![image](https://github.com/user-attachments/assets/1b9c65ec-9bca-4a84-a742-68b8109d039c)

### Demo - after
<img width="584" alt="image"
src="https://github.com/user-attachments/assets/043842cd-6b27-4511-aa5d-6941cb79420f"
/>



<!-- Required. See
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles.
-->
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