[Workbench] Sync with selected open file#1576
Conversation
add sidebar callback and header
aligned static header definition with static implementation
|
From my limited knowledge of geany plugin and gtk development I think this looks good so far aside of the two minor points I commented on in the files. |
I reviewed the pull request looking for comments, but github isn't showing me anything. I don't normally use github, so I am not sure if I am looking in the wrong place, or if it only shows comments to the pull reviewers. I welcome any feedback though, as I haven't written C since the 90s and just started GTK last week. |
| /* it seems odd to assert this here, | ||
| * as an untitled doc (new one) would have no filename. | ||
| */ | ||
| g_return_if_fail(doc != NULL && doc->file_name != NULL); |
There was a problem hiding this comment.
Your comment states uncertainty if this should be here or not. Has this been clarified yet? If there is actual reason to check for the file_name, it should probably be specified in the comment as for why this is done. Otherwise, if it works without this filename check, maybe removing it is the better option to not cause more confusion.
There was a problem hiding this comment.
It seems confusing because I am confused. I copied this guard to honor the pattern established by the project maintainer. Until I get clarification, I am afraid to remove it, as I have already had to close one pull due to changing something that actually turned out to be load-bearing elsewhere.
sorry, it seems github doesn't automatically puts those comments in the PR but requires me to actually "finish" the PR-review. I haven't used githubs' review system before and this wasn't quite clear to me as the comments even appeared here in the comment section. |
|
Both your comments sound reasonable and as I won't be able to tell you how both of them could be tested, I'd say it's good enough for now so I would approve it in this state but maybe add some cleanup tasks so those topics may get clarified sometime in the future. But that's just what I would recommend, not sure if there are already rulings for geany plugin development that define a certain path to follow here. |
|
Moved this so I could change the Pull request to About this make is clean
geany verbose log has a lot of messages, but unrelated to my changes Closes #1227 |

About this:
As requested in #1227 this change adds bi-directional file-tree select to Workbench.
When clicking on a file in the file tree, if that file is open, that tab is selected.
When clicking on a tab in the editor, if that file exists in the file-tree, it is scrolled to (vertically only), and selected.