This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
I would like to suggest a patch that adds a separate class which takes care of painting the code folding. It's a simple public SPI with one method. The class is org.netbeans.editor.view.spi.CodeFoldingPainter with the paint(Graphics g, PaintInfo info,int markSize, Font defFont, List visibleMarks) method, which is basically a refactored method for what goes on in CodeFoldingSideBar.paintComponent(). CodeFoldingPainter is defined per mime-type so we have the possibility of different fold GUIs depending on mimetype. No layer changes are required so far, it uses the default painter if nothing is defined in the MimeLookup. If it is defined (meaning that an .instance file was added for example) it will use that. The patch is against the mercurial e4cac6efdbe1 changeset (a week old at this moment). It also includes the fixes provided here http://www.netbeans.org/issues/show_bug.cgi?id=144296 for another small folding issue (a bug in the default fold GUI). Please note that another enhancement is included here (not sure if I should post is separatelly). If a Mark with the Position.Bias.Backward bias (the Mark class is changed a bit, see patch) then we basically have the support for a "backward" click. That is, we can collapse a fold by clicking on a GUI artifact at the end of it (see attached screenshot and demo implementation).
Created attachment 68868 [details] editor.lib patch
Created attachment 68869 [details] Demo screenshot of new GUI with backward collapse working.
Created attachment 68870 [details] Code for the triangle GUI with the backward collapse.
Thanks for the patch, but I am not sure if I understand the motivation for CodeFoldingPainter. Currently CodeFoldingSideBar is publicly accessible and modules have to register it as a sidebar (ie. under Editors/<mime-type>/SideBar). They are allowed to register any other implementation which can render folds in its own way. So, everything that you are proposing for CodeFoldingPainter is already available. Or am I mistaken?
True, but you would have to duplicate a lot of the original code if you want just a new GUI for folding. Anyhow, maybe you'll find the enhancement for "backward" folds good enough to include in main.
> True, but you would have to duplicate a lot of the original code if you want just a new GUI for folding. You could just subclass CodeFoldingSideBar and override its paintComponent, could you not? Anyway, I mean, having the whole CodeFoldingSideBar in the API as it is now is terrible and your CodeFoldingPainter looks much better from the API standpoint. So, I'm not against it. I'm just asking questions that people are likely to ask on the apireviews mailing list. > Anyhow, maybe you'll find the enhancement for "backward" folds good enough to include in main. Yes, I like that. It's pretty smart. I'm CCing our UI guru Ondra to here his opinion. And if there are no objections we can add it. Actually, we can't add it to 6.5, because we have already passed UI freeze milestone, but we should certainly be able to add it to next version.
Petre, this old request proposes a change to appearance of code folds. Functionally, we do not need 'backward folding' as now it is possible to click on the fold outline to collapse the inner-most fold. Please evaluate the visual appearance of this alternative. Also see e.g. IntelliJ, which uses similar down+up arrow pairs to mark start/end of fold.
Created attachment 119954 [details] Proposed thickening of fold on mouse over The GUI shown in attachment http://netbeans.org/bugzilla/attachment.cgi?id=68869 is not satisfactory from UEX point of view because in NetBeans context id adds too much visual clutter. In IntelliJ this is less of a problem because the folds themselves are used as the left edge of the editor window. Generally it is visually easier to recognize the extent of foldable blocks when the beginning of the block is marked in significantly different way from the end. Therefore, based on the fact that we now support double-clicking on the fold outline to collapse the inner-most fold, I vote against the introduction of backward folds as illustrated in the attachment. One detail needs to be corrected though in the current implementation. Double-click works only if targeted inside the fold outline but not at its terminating "corner". Double-clicking the corner does nothing, but it should collapse the fold to which it belongs. The fact that it currently does nothing decreases discoverability, which in itself is a kind of problem with the fold double-click functionality. However, due to the discoverability issue, I suggest to change fold color on-mouse-over (meaning the upper rectangle with minus sign and the whole line including the corner that belongs to the fold in question). This would not only suggest to the user that the fold is active and thus probably clickable, it would also emphasize the extent of that particular fold. Now the folds are dark gray; I do not know how does it depend on LaF, but in general the on-mouse-over color should be not too striking but it should be notable that it is different from the default one (especially it should be good enough to distinguish close corners of several overlapping folds). Alternatively, instead of a different color it might be better to draw the current on-mouse-over fold by one pixel wider, what would be visible in all LaFs and by more people (even those with partial/full color blindness). I am attaching a mockup of how the inner fold could look like with mouse cursor over it (mouse cursor not included in picture).
Incidentally ;)) I have already implemented the bold line on mouse hover; the feature is ready for 7.3. Will try to fix the click behaviour for terminating mark.
(In reply to comment #8) I don't see how the NetBeans visual clutter would be any more different than Intellij IDEA's. I wasn't aware about the double-click, so I don't know how discoverable that is. But a simple mark at the end of the fold that you could click on is truly self-obvious. I like the idea of the bold line on mouse over. Still -- what you aren't noticing on my patch is that it makes the fold painting a service. All you guys would need to do is support an API and provide the default fold painting (which need not have a backward collapse). But if the API is there I can easily install a module providing another folding painter that *does* have backward collapse similar to IDEA's.
(In reply to comment #10) > Still -- what you aren't noticing on my patch is that it makes the fold > painting a service. All you guys would need to do is support an API and provide > the default fold painting (which need not have a backward collapse). > > But if the API is there I can easily install a module providing another folding > painter that *does* have backward collapse similar to IDEA's. My comments related only to UEX. As for the API proposal, I can only tell that I endorse any viable possibility of providing custom implementation. Comment 8 should apply to the default implementation but it would be good to enable users to replace it with whatever they want. Whether it is through the "old" way or through the new API, I leave it to Svata to decide.
Let me say that PaintInfo si a terrible API; it's rather an internal structure and should have been never published. Even a small visual change typically lead to adjustments or extension of the PaintInfo structure. So it will either change, or Painter need to perform additional processing to complete its information. If I leave out Painters, which display different information - for which they would need either different PaintInfo, or create additional data in addition to PaintInfo, we are talking about "just" changing the glyphs. It's true that NB aim to be extensible, but each API increases the maintenance costs, so we need to choose important stuff for an API. Important means such APIs, which are valuable to several clients to cover core their use-cases. The issue of replacing glyphs per MIME type, while leaving functionality as it is does not seem as one of such cases. So I am not in favour of creating an additional layer of painting. In fact I would like to deprecate the Sidebar API in the future and remove the code from public packages entirely after several releases. BTW - why should the presentation of folds vary for different MIME types ? The concept is the same, the other editor decorations apply for all file types ... what is the use-case for altering the code folding glyphs ?
(In reply to comment #11) > My comments related only to UEX. IDEA-style code folding is quite pleasant. I fail to see how you think it's cluttered from an UX standpoint...
(In reply to comment #12) There are 2 issues here: * having functional backward collapse "glyphs" -- you will solve this by making the whole fold click-able (I guess, in the next 7.3 or 7.2?). I presume the ending (L-shaped) glyph is click-able too? * having a configurable UI for the folding glyphs and line. Having a different UI means we need some sort of API to make it happen. I can't just copy-paste the whole folding sidebar just to tweak the UI. So some "painter API" would be nice, even if it's more polished than just using the raw PaintInfo. It's actually unfortunate that an API has to be an all-or-nothing proposition. There are general purpose APIs and niche APIs and I think they should not follow the same rules. It's acceptable for niche APIs to be less stable. My only choice thus remains to continue using actual source code patches and compiling my own Platform. PS: Using the MimeLookup or the global Lookup doesn't really change the discussion.
(In reply to comment #14) > the whole fold click-able (I guess, in the next 7.3 or 7.2?). I presume the > ending (L-shaped) glyph is click-able too? Not yet, but will be. The whole thing hopefully becomes live in 7.3, it's almost too late to fiddle with 7.2 UI. > It's actually unfortunate that an API has to be an all-or-nothing proposition. As for stability yes. I could refactor the source so that your module could eventually use impl. dependencies to inject something; maintenance burden then falls on your head :) But I'd prefer not to publish niche APIs just because of maintenance overhead. Still unexplained issue is why should different MIME types use different presentation; specific use-cases could help here, where to draw the "api" division line.
> Not yet, but will be. The whole thing hopefully becomes live in 7.3, it's > almost too late to fiddle with 7.2 UI. I guess that's progress, but it would have been nice to backport this to, say, 7.1. > As for stability yes. I could refactor the source so that your module could > eventually use impl. dependencies to inject something; maintenance burden then > falls on your head :) But I'd prefer not to publish niche APIs just because of > maintenance overhead. This would be great! > Still unexplained issue is why should different MIME types use different > presentation; specific use-cases could help here, where to draw the "api" > division line. The "painter" is global anyhow. I don't remember why I picked the Mime Lookup. I guess because it seemed more flexible to me :-) If I wanted, I could in theory have different painters per mimetype but that would look weird.
OK, so the actual requested adjustments are: * allow to replace glyphs painted by the CF toolbar (various types of mark and lines) * globally replace the painter. Perhaps Lookup.getDefault() for consistency. For that, a more restricted API could be suitable. Say if the Painter is fed instructions, in top-down order, to paint line, glyph, line, glyph, it can draw the joins / branches at will, all computation will remain in the core sidebar. The 'glyph' painting method could get parameters: * glyph type * state * nesting level * Y- position (+ Y2 end position for lines) * whether the glyph is joined up and/or down with line * whether there is a continuation line The Y-positions would be always measured to the top of the line. PaintInfo :) sort of. I would prefer that the painter instance is instantiated for the paint job, then discarded, so it can keep some painting state/context.
(In reply to comment #17) > OK, so the actual requested adjustments are: > * allow to replace glyphs painted by the CF toolbar (various types of mark and > lines) > * globally replace the painter. Perhaps Lookup.getDefault() for consistency. Yes. > [...] PaintInfo :) sort of. That does look strangely similar to PaintInfo ;) > I would prefer that the painter instance is instantiated for the paint job, > then discarded, so it can keep some painting state/context. Makes sense, although not necessary right now. The painter seems to work even stateless. In this case you would need a PainterFactory in the Lookup.
This old bug may not be relevant anymore. If you can still reproduce it in 8.2 development builds please reopen this issue. Thanks for your cooperation, NetBeans IDE 8.2 Release Boss