Skip to content

Conversation

@HeikoKlare
Copy link
Contributor

While actions of the target editor are properly deactivated when the target is an ordinary, single-page editor, the same does not happen when the target is a multi-page editor. In consequence, for example, you cannot paste clipboard content into the overlay via the according keyboard shortcut (CTRL+V), but it will paste into the target editor instead.

With this change, the functionality to deactivate target editor actions is extended to also deactivate the relevant global actions (cut, copy, paste, etc.) that editors like multi-page editors fall back to.

Fixes to #2509

Note

I hope there is a better solution to deactivate the according action handlers in a multi-page editor than this, but I did not find it so far. Still, I think this is better than not being able to use common shortcuts for cut, copy, paste etc. when using multi-page editors.

Behavior

Without the fix:
findreplace_multipage_before

With the fix:
findreplace_multipage_after

@HeikoKlare HeikoKlare marked this pull request as ready for review November 12, 2024 16:02
@HeikoKlare
Copy link
Contributor Author

@HannesWell what do you think?

@github-actions
Copy link
Contributor

github-actions bot commented Nov 12, 2024

Test Results

 1 821 files  + 1 821   1 821 suites  +1 821   1h 55m 12s ⏱️ + 1h 55m 12s
 7 725 tests + 7 725   7 496 ✅ + 7 496  228 💤 +228  1 ❌ +1 
24 336 runs  +24 336  23 588 ✅ +23 588  747 💤 +747  1 ❌ +1 

For more details on these failures, see this check.

Results for commit 5c7a21c. ± Comparison against base commit 64af72a.

♻️ This comment has been updated with latest results.

Copy link
Member

@HannesWell HannesWell left a comment

Choose a reason for hiding this comment

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

Thank you for this fix! I really appreciate that.

I tested it and it works as desired, but I cannot tell if there is a generally better way to implement this.

Since I think this is an important fix, I approve this for submission for RC1 as PMC member:
PMC +1

@HannesWell HannesWell added the pmc_approved Issues with PMC approval label Nov 12, 2024
…tform#2509

While actions of the target editor are properly deactivated when the
target is an ordinary, single-page editor, the same does not happen when
the target is a multi-page editor. In consequence, for example, you
cannot paste clipboard content into the overlay via the according
keyboard shortcut (CTRL+V), but it will paste into the target editor
instead.

With this change, the functionality to deactivate target editor actions
is extended to also deactivate the relevant global actions (cut, copy,
paste, etc.) that editors like multi-page editors fall back to.

Fixes to
eclipse-platform#2509
@HeikoKlare
Copy link
Contributor Author

Thank you for the feedback and the approval, Hannes!

By accident, I had to search through several Manifests right after proposing this PR and experienced how annoying the issue is. Seems like I had no need to search in Manifests for quite a while. So now I definitely agree that this should have high priority and that it should go into the upcoming release.

@HannesWell
Copy link
Member

So now I definitely agree that this should have high priority and that it should go into the upcoming release.

Yes, absolutely 😅
Please submit this as soon as the build has completed.

@HeikoKlare
Copy link
Contributor Author

Failing test is unrelated: #370

@HeikoKlare HeikoKlare merged commit 69f6e45 into eclipse-platform:master Nov 12, 2024
14 of 17 checks passed
@HeikoKlare HeikoKlare deleted the issue-2509-2 branch November 12, 2024 19:18
@BeckerWdf BeckerWdf added this to the 4.34 RC1 milestone Nov 14, 2024
@chrisrueger
Copy link
Contributor

@HeikoKlare I have this problem in Bndtools / BndEditor with Eclipse 2024-12 which under the hood is a FormEditor which is a MultiPageEditorPart.

https://github.com/bndtools/bnd/blob/9ed91bd967559c0bf963753990e96719bf6ea4cb/bndtools.core/src/bndtools/editor/BndEditor.java#L104

It happens in the Source tab (which is a org.eclipse.ui.forms.editor.FormPage extends EditorPart implements IFormPage https://github.com/bndtools/bnd/blob/9ed91bd967559c0bf963753990e96719bf6ea4cb/bndtools.core/src/bndtools/editor/pages/BundleContentPage.java#L39)

public class BndEditor extends ExtendedFormEditor implements IResourceChangeListener, IBndEditor {

public abstract class ExtendedFormEditor extends FormEditor {

public abstract class FormEditor extends MultiPageEditorPart {

Is this a different / new case which needs to be handled? (and maybe added to the umbrello issue #2021)

Or is there something Bndtools should do, to fix it?

@merks
Copy link
Contributor

merks commented Mar 6, 2025

@chrisrueger

I fixed some related problems during the current release cycle because I noticed the PDE target editor was working poorly.

I would suggest testing with 2025-03 / 4.35, though it's too late now to fix anything for that release. Any fixes will have to happen for 2025-06 / 4.36...

@chrisrueger
Copy link
Contributor

chrisrueger commented Mar 6, 2025

I would suggest testing with 2025-03 / 4.35,

I just tested with 2025-03 / 4.35 RC1 and I confirm the problem in Bndtools Editor is fixed 👍
Great work @merks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pmc_approved Issues with PMC approval

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants