Filter for symbol tree - improved version

[techee]

I took the symbol tree filtering implementation from #2657 and applied some of the suggested changes on top of it:

• made the filter entry per-document
• made filtering case-insensitive
• cleared the symbol tree completely when filtering to ensure it's fully re-created
• filtering using full tag name including scope
• focusing the symbol tree after pressing enter in the search entry

What do you think?

[eht16]

Except two minor remarks, this is great! Tested and works very well.
I'd expected that it required more code to make the filter per document but happy to be proven wrong :D.

[elextr]

Gentle prompt, its a UI change and new functionality so manual change also needed.

[techee]

Except two minor remarks, this is great!

Where are those? Since I can't find them, maybe a minor remark of my own :-).

Maybe we should consider what widget in the sidebar gets the focus by default - right now it's the entry (and the tree focus is just one tab or enter press away) but maybe users who use primarily keyboard are more used to having the tree focused. I don't mind much myself as I just use mouse for the tree.

I'd expected that it required more code to make the filter per document but happy to be proven wrong :D.

Same here and it actually applies to the whole patch. I was thinking about this feature for a long time but thinking it would be annoying to do and then saw the pull request which was pretty trivial and mostly working so I got attracted :-).

Gentle prompt, its a UI change and new functionality so manual change also needed.

Sure, if it's something that's going to be merged, I'll definitely update it.

[eht16]

Except two minor remarks, this is great!

Where are those? Since I can't find them, maybe a minor remark of my own :-).

Sorry, I forgot to submit my review. Now done.

Maybe we should consider what widget in the sidebar gets the focus by default - right now it's the entry (and the tree focus is just one tab or enter press away) but maybe users who use primarily keyboard are more used to having the tree focused. I don't mind much myself as I just use mouse for the tree.

Same here and not sure what would be the best default.

I'd expected that it required more code to make the filter per document but happy to be proven wrong :D.

Same here and it actually applies to the whole patch. I was thinking about this feature for a long time but thinking it would be annoying to do and then saw the pull request which was pretty trivial and mostly working so I got attracted :-).

:)

[techee]

Gentle prompt, its a UI change and new functionality so manual change also needed.

I actually haven't found anything in the documentation regarding the symbol tree (unless I missed it) so I'd have to write some new section about it. In my opinion, the tooltip says it all and there shouldn't be any need for any extra explanation.

Maybe we should consider what widget in the sidebar gets the focus by default - right now it's the entry (and the tree focus is just one tab or enter press away) but maybe users who use primarily keyboard are more used to having the tree focused. I don't mind much myself as I just use mouse for the tree.

I kept the focus as it is now - it seems to behave quite OK and and it would require moving

geany/src/keybindings.c

Line 1700 in 0601c73

static void focus_sidebar(void)

to sidebar.c so it has access to the tree to call gtk_widget_grab_focus() and also updating

geany/src/sidebar.c

Line 1143 in 0601c73

void sidebar_focus_symbols_tab(void)

None of it is a problem to do though if someone wants to focus the tree by default.

[eht16]

I'm happy with current focus behavior and also with the rest.
Great.

I'll leave @elextr a bit time to raise his veto, otherwise I'd like to merge soon.

[elextr]

Ok, so we have abandoned the manual then? (This is not directed at @techee specifically, it is a general question, a number of recent changes have not been documented, but then thats what the code is for, right?)

Every time something is merged without documenting it "because there is nothing in the manual at the moment" is another time the manual becomes more useless to users.

[end grumpy]

[techee]

Every time something is merged without documenting it "because there is nothing in the manual at the moment" is another time the manual becomes more useless to users.

I was just asking, I can write something if desired. I just haven't found any place where I should put it and none of the items in the right-click menu in symbol tree is documented either right now. The searchbar in the plugin manager isn't documented either.

I'm not sure if documenting obvious things like filtering just doesn't add noise to the documentation without any real value.

[elextr]

I was just asking

Sorry if you took it personally, as I said, my grump wasn't directed at you, we are all responsible for it.

I'm not sure if documenting obvious things like filtering just doesn't add noise to the documentation without any real value.

Its a good question, what are we targeting the manual at?

Its currently a random mixture of beginners info and experts info. Certainly Geany users are mostly some sort of developer (I count Markdown and similar writers as developers), but not necessarily the same type, a C expert may not be a HTML/web expert and vice versa. So things that are "obvious" to some users may not be obvious to others even if they are not "beginners", so at least mentioning capabilities to make them discoverable would be good.

As an example I would just add a subsection to the end of the "The Geany workspace" section that mentions the capabilities without going into chapter and verse about how to invoke them or needing @eht16 to add and maintain an image of his right click menus 😄.

Sidebar Usage
^^^^^^^^^^^^^

The sidebar has a right click menu that can control what is visible and has actions specific to the tab (other tabs added by plugins are described by that plugin documentation):

* Symbols Tab
  * expand/collapse the tree
  * control sorting order
  * locate the symbol in documents

  The symbols tab can also be filtered by typing a symbol prefix into ... [@techee to fill in, don't forget when the filter autoremoves (if it does)]

* Documents 
  * expand/collapse the tree
  * save to or reload from files
  * search tree based at selected file
  * show or hide the document paths

(Warning: my rEsT may need correcting)

[eht16]

Ok, so we have abandoned the manual then? (This is not directed at @techee specifically, it is a general question, a number of recent changes have not been documented, but then thats what the code is for, right?)

IMO "abandoned" is a bit too strong here. I try to remind myself and others of updating the manual but sometimes I consider additions also as not worth adding or just obvious while being aware that "obvious" is very subjective.
Maybe we should pay some more attention to it but we also should try to not extend the already existing burdens for getting PRs created/merged even more.

I'm not sure if documenting obvious things like filtering just doesn't add noise to the documentation without any real value.

Its a good question, what are we targeting the manual at?

I guess the target audience is what you described: users with a very beginner level as well as "C experts". I think we can not and should not decrease the target audience.

In this case, I would agree with @techee that the filter feature (with its tooltip) is pretty much obvious.
But since you already provided a good stub for a new section in the manual, it's enough to use and extend it. Great!

[techee]

Sorry if you took it personally, as I said, my grump wasn't directed at you, we are all responsible for it.

No problem at all, I just wasn't sure what to do.

I'll use your stub (thanks) and update the docs using it.

[elextr]

IMO "abandoned" is a bit too strong here.

Sorry, but grumpy olde guys are required to go a bit overboard, its in our contract 😄

users with a very beginner level as well as "C experts".

Because I have been trying other tools recently where I am what I would call an "expert beginner" I can appreciate how useful it is having at least a mention of features in the help/manual to point in the right direction.

Things like filters behave differently on different tools, neither Eclipse or Vscode have filters on symbols (AFAICT)¹, and for example the Eclipse filter for the equivalent of the "Documents" sidebar has a tick list to filter by "C/C++ Files" and categories like that, so having an outline of what the Geany one does is worthwhile to reduce expectations and "how do I do XXX" issues (they become "feature request" issues instead 👿 ).

but we also should try to not extend the already existing burdens for getting PRs created/merged even more.

I think sometimes PR writers overestimate what I expect by "documentation" and I appreciate that English isn't everyone's native language, thats why I provided the outline as an example to show what I personally think is useful.

So long as there is some outline or mention of the feature, at least it indicates that there is some functionality in that area, where it is hiding (eg Eclipse document filter is hidden behind a funnel icon on the toolbar, not an entry like Geany symbols filter), and will show up when help is searched, which is totally sufficient for "expert beginners" and somewhere for "beginner beginners" to start from.

Remember the Geany "Manual" is also its "Help" and as it opens in a browser there is usually good search capability.

Rest assured I have no intention of turning the Geany Manual into a full 1000 page book 😄

The outline can also be added to when the "Documents" filter is added.

Footnotes

1. Edit: I just found the eclipse one hidden behind a hamburger, again it has a tick list of categories as well as a names entry ↩

[eht16]

Docs look fine to me.

[elextr]

Generally works for me for C, but there is a problem with Asciidoc and Rest where stray headings are catenated by 0003 characters, to reproduce open geany.txt and filter on re.

[techee]

Generally works for me for C, but there is a problem with Asciidoc and Rest where stray headings are catenated by 0003 characters, to reproduce open geany.txt and filter on re.

Good catch, I was thinking about it but then forgot to implement it. I added tm_parser_scope_separator_printable() which returns a printable representation of scope separator (for most languages identical to tm_parser_scope_separator()). I cleaned up the scope separator calls a bit on the way as it was easy to get confused by their different versions.

[elextr]

@techee why do we have conflicts?

[techee]

This is because #3060 was merged and I used tm_parser_scope_separator_printable() also inside the calltip formatting function that was affected by that pull request.

How should I resolve the conflict - merge master to this branch, rebase, or possibly submit the last 4 patches as a separate pull request?

[elextr]

This is because #3060 was merged

Ahh, tripped yourself up

Probably this is a case where there is no choice but to rebase this (but not Git expert).

[techee]

I did a hybrid approach - did reset to HEAD~4 so the branch contained only the tag filter feature without any conflicts, then rebased it on top of master, and then re-wrote the last 4 patches manually - it was simpler than rebasing the whole branch.

[eht16]

Still LGTM, the stray headings problem seems solved.

Fine to merge?

[elextr]

I can't test anything for a while, but if ReSt and Asciidoc are fixed its fine with me.

[eht16]

I just tested the filtering for "re" in geany.txt as you mentioned above, this works. Though I don't have any Asciidoc files around to test with.

[elextr]

It was the same problem IIUC, so if rest is ok, asciidoc should be ok too (except for bugs :-) so don't wait for me.

[eht16]

Great.

@techee do you like to squash some of the commits?

[techee]

@techee do you like to squash some of the commits?

Yeah, will do for the obvious "bugfix" ones.

[techee]

Yeah, will do for the obvious "bugfix" ones.

Done.

Though I don't have any Asciidoc files around to test with.

All of the non-printable scope separators (and "strange" separators like \"\") are handled by

bd93be4

the same way so when it works for rst, the same will work for Asciidoc too.

[eht16]

Great! Thanks.