Skip to content

Enable switching preview for search results, refactoring and bookmarks#8326

Merged
matthiasblaesing merged 2 commits intoapache:masterfrom
matthiasblaesing:preview-disable
Apr 5, 2025
Merged

Enable switching preview for search results, refactoring and bookmarks#8326
matthiasblaesing merged 2 commits intoapache:masterfrom
matthiasblaesing:preview-disable

Conversation

@matthiasblaesing
Copy link
Copy Markdown
Contributor

Search results (multi file search), refactoring (search usages, renaming) and bookmarks all have preview panes, that allow users to see the context for the selected entry:

grafik

When this was added to search usages via #7694 there was already the idea to make the preview toggleable. This PR adds toggles for preview to all the locations:

Refactoring in general (useful in "Find usages", less so in "Renaming", but setting is stored per use-case, so user is free to disable preview for "Find usages" and enable it for "Renaming"):

grafik

grafik

Search results

grafik

Bookmarks

grafik

Closes: #8112

@matthiasblaesing matthiasblaesing added Platform [ci] enable platform tests (platform/*) ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) labels Mar 12, 2025
@matthiasblaesing matthiasblaesing added this to the NB26 milestone Mar 12, 2025
@dschniedertuens
Copy link
Copy Markdown

The switch is great!
I tested "Find in Projects" and "Find Usages". The "Search results" pane works exactly as I would expect.
However the "Usages" pane doesn't remember the last switch state.
Also it sometimes doesn't enable the preview although the button state indicates that it is enabled:
image

@mbien
Copy link
Copy Markdown
Member

mbien commented Mar 16, 2025

haven't had time yet to look through everything yet, but I found a problem with the refactoring preview, see fix in:
https://github.com/mbien/netbeans/commits/preview-disable/

first commit might be squashable with the cleanup commit, second commit has the fixes, see msg. (feel free to squash)

@matthiasblaesing
Copy link
Copy Markdown
Contributor Author

@dschniedertuens-tc thanks for testing, indeed the implementation for the refactoring panel was off. An updated version is available here: https://github.com/apache/netbeans/suites/35751671606/artifacts/2760443851

@mbien thanks for testing. I plan to squash the cleanup commit into the first cleanup (there is a conflict, but that is trivial to resolve). The second commit more or less matched my initial thoughts after looking into the initial testing report, however I added some more fixes so went with my already started work.

@dschniedertuens
Copy link
Copy Markdown

I can confirm that the preview switch for the usages panel now also works as I would expect.
Thank you for your work on it!

@mbien
Copy link
Copy Markdown
Member

mbien commented Mar 17, 2025

this avoids out of bounds issues:

diff --git a/ide/diff/src/org/netbeans/modules/diff/builtin/visualizer/editable/DiffViewManager.java b/ide/diff/src/org/netbeans/modules/diff/builtin/visualizer/editable/DiffViewManager.java
index 40ecfd9..42df713 100644
--- a/ide/diff/src/org/netbeans/modules/diff/builtin/visualizer/editable/DiffViewManager.java
+++ b/ide/diff/src/org/netbeans/modules/diff/builtin/visualizer/editable/DiffViewManager.java
@@ -505,7 +505,7 @@
             if (checkFileEdge && rightOffet >= rightPane.getScrollPane().getVerticalScrollBar().getMaximum()) {
                 rightOffet = map.length - 1;
             }
-            if (rightOffet >= map.length) return;
+            if (rightOffet >= map.length || rightOffet < 0) return;
             leftPane.getScrollPane().getVerticalScrollBar().setValue(map[rightOffet]
                     - halfScreen);
         }

This happens when preview is off and the refactoring view changes selection. The diff panel tries to scroll even though it is not visible. This sounds like unnecessary work given that it is not visible but i think it is probably ok, because if you toggle it on again it will show at the right position - which is what you would expect from it I think.

Details
java.lang.ArrayIndexOutOfBoundsException: Index -1 out of bounds for length 0
	at org.netbeans.modules.diff.builtin.visualizer.editable.DiffViewManager.smartScroll(DiffViewManager.java:509)
	at org.netbeans.modules.diff.builtin.visualizer.editable.DiffViewManager.access$300(DiffViewManager.java:47)
	at org.netbeans.modules.diff.builtin.visualizer.editable.DiffViewManager$2.run(DiffViewManager.java:154)
	at java.desktop/java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:318)
	at java.desktop/java.awt.EventQueue.dispatchEventImpl(EventQueue.java:723)
	at java.desktop/java.awt.EventQueue.dispatchEvent(EventQueue.java:702)
	at org.netbeans.core.TimableEventQueue.dispatchEvent(TimableEventQueue.java:136)
[catch] at java.desktop/java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:203)
	at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:124)
	at java.desktop/java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:113)
	at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:109)
	at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
	at java.desktop/java.awt.EventDispatchThread.run(EventDispatchThread.java:90)

@mbien
Copy link
Copy Markdown
Member

mbien commented Mar 17, 2025

added another commit for the bookmarks preview mbien@27e652e, feel free to squash or take bits of it.

@neilcsmith-net
Copy link
Copy Markdown
Member

Could you take a look at #8350 Might be relevant to changes here.

Co-authored-by: Michael Bien <mbien42@gmail.com>
@matthiasblaesing matthiasblaesing force-pushed the preview-disable branch 2 times, most recently from c9c13f8 to 3235c61 Compare March 28, 2025 18:47
@matthiasblaesing
Copy link
Copy Markdown
Contributor Author

@mbien thank you for the updates, which I integrated
@dschniedertuens-tc it would be great if you could give the nightly a spin: https://github.com/apache/netbeans/suites/36384122149/artifacts/2841267690
@neilcsmith-net I had a very quick look at #8350, but decided not to make this more complex as is. #8350 is a different beast, but
related.

Assuming check become green and or anybody raises a veto, I plan to merge this by end of next week.

Copy link
Copy Markdown
Member

@mbien mbien left a comment

Choose a reason for hiding this comment

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

tested: find usages, search, bookmarks and refactoring windows and everything worked - changes looked good to me too

left one comment for you to consider

Co-authored-by: Michael Bien <mbien42@gmail.com>
@mbien mbien added Editor UI User Interface labels Mar 31, 2025
@dschniedertuens
Copy link
Copy Markdown

I've been working with the linked nightly for the last few days now, constantly using search and usages and the switch still works very well for me. Now that it can be disabled, I have even found cases for which I want to enable the preview 😉
Many thanks for the great work!

@matthiasblaesing
Copy link
Copy Markdown
Contributor Author

Lets get this in. @mbien thanks for review and suggestions, @dschniedertuens-tc thanks for testing.

@matthiasblaesing matthiasblaesing merged commit d34286d into apache:master Apr 5, 2025
32 checks passed
@matthiasblaesing matthiasblaesing deleted the preview-disable branch April 9, 2025 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) Editor Platform [ci] enable platform tests (platform/*) UI User Interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Usages: Add means to disable code preview

4 participants