Add Pinner plugin

[andy5995]

From what I can tell, as I'm just learning the Geany API and gtk3, I would now need to

1. Create a linked list using GSList() to store each item that's pinned
2. Before pinning, check to see if document is already in the list
3. Other stuff after that.

I'm a bit lost on how to pass some values as functions though. I'm not too experienced with callback functions and some of my experiments have gone terribly awry.

I'll ask in a line comment...

[eht16]

Uhhh, that sounds interesting and like something I want to implement since some years already :).

[eht16]

Uhhh, that sounds interesting and like something I want to implement since some years already :).

I just found some old code and it seems I started with a similar approach some time ago: https://gist.github.com/eht16/15afd001f238f77f100a43e4129b8d20
Use what is helpful but with care 😄. I don't remember much its state except that it was working to some extend but quite buggy and probably also crashed happily.

[andy5995]

What I have so far works as expected.

[andy5995]

Duplicate checking works now. Any feedback on how I'm implementing this so far?

One thing I'm not sure of yet is how to pass the the list to the cleanup function so I can free it.

[andy5995]

One thing I'm not sure of yet is how to pass the the list to the cleanup function so I can free it.

I made the list global.

[andy5995]

I just found some old code and it seems I started with a similar approach some time ago: https://gist.github.com/eht16/15afd001f238f77f100a43e4129b8d20 Use what is helpful but with care 😄. I don't remember much its state except that it was working to some extend but quite buggy and probably also crashed happily.

@eht16 It looks like we were working on something different but similar? ;) I think incorporating something like that into this plugin would be suitable.

[andy5995]

At this point, clicking on a document in the "Pinned" tab, will open (switch to) it.

[andy5995]

Ready for review and testing

[luigifab]

Do you have a final screenshot?

[andy5995]

[andy5995]

Longer filenames are now handled better, and they're left-justified with a 10 pixel margin on both sides.

@luigifab

[andy5995]

"Unpin Document" available by right-clicking

[andy5995]

I moved this to https://github.com/andy5995/pinner

[eht16]

@eht16 It looks like we were working on something different but similar? ;) I think incorporating something like that into this plugin would be suitable.

Yes, my approach was for a different use case. I like to have a tab/document always in the visible area of the notebook tab list for easy access. The name of your plugin sounded like this to me in the beginning :).

[elextr]

Couple of comments:

1. write the documentation explaining what this plugin is supposed to do, sounds like it does something different to what @eht16 understood "pin" to mean, and I have always been confused about what it was meant to do. (and do not describe it in terms of some other software, eg do not say "like firefox/eclipse/vscode/libreoffice" ;-) [end getting in ahead] That will mean that you can get relevant guidance.

2. relating to Return NULL from document_get_current() if document is 'untitled' geany#3770 and depending on the answer to the above, you should be working with notebook pages, not documents. I'm pretty sure that plugins can open pages that do not have documents, let alone files.

3. And a hint always check document_valid() on a document pointer in any callback if you pass it via the data pointer, the user can always close pages and documents or reorder pages manually.

[andy5995]

write the documentation explaining what this plugin is supposed to do, sounds like it does something different to what @eht16 understood "pin" to mean, and I have always been confused about what it was meant to do.

I'm happy to elaborate an idea or re-frame a goal if I haven't made myself clear to someone. In this case, I didn't know I wasn't being understood. Please let me know next time if you are confused by my description or feel I need to expand on it more.

and do not describe it in terms of some other software, eg do not say "like firefox/eclipse/vscode/libreoffice" ;-)

I feel that's an unnecessarily rigid policy and I don't really plan to stop using them (when I believe it's appropriate) based on your demand. I believe that comparisons can be a useful tool when trying to describe something; but I acknowledge that sometimes comparisons aren't always adequate and shouldn't be solely relied upon to express a desired feature.

[end getting in ahead]

I don't know what that means.

relating to Return NULL from document_get_current() if document is 'untitled' geany#3770 and depending on the answer to the above, you should be working with notebook pages, not documents. I'm pretty sure that plugins can open pages that do not have documents, let alone files.

I don't know what you mean. I work with documents when I use Geany. The plugin apparently uses a sidebar notebook (plugin->geany_data->main_widgets->sidebar_notebook). I use the plugin to open or switch to documents. Maybe our miscommunication stems from my lack of experience developing Geany or GTK development, but feel free to elaborate.

And a hint always check document_valid() on a document pointer in any callback if you pass it via the data pointer, the user can always close pages and documents or reorder pages manually.

I searched for document_valid in the geany-plugins source code and no results came up. When I searched for document_get_current, many results came up. I'm confused as to why you would suggest I use a function that no other plugin developer uses... and apparently isn't even a function included in the API.

I'm not accessing any GeanyDocument pointers passed through a callback. In one case, I pass the file_name member as a gchar, after it's been validated by another function.

Due to #3770 I use this to validate the file_name after I enter a callback function:

  GeanyDocument* doc = document_get_current();
  if (!(doc && doc->is_valid))
    return;

  if (doc->file_name == NULL)
    return;

[andy5995]

I forgot to mention, based on your feedback @elextr , I expanded the README.md with hopefully a better explanation of the goal of the plugin, and added a link to a demo video.

[elextr]

but I acknowledge that sometimes comparisons aren't always adequate and shouldn't be solely relied upon to express a desired feature.

Yes, that was what I was asking, describing a feature as "I want it to work like Vscode" is unhelpful if the reader doesn't use Vscode or that feature of Vscode, whereas "I want it to do xyz when I click blah because I have found it useful in Vscode" is of course fine.

[end getting in ahead]

I was simply saying that I was making the above point as guidance ahead of you doing anything, not that it was a criticism of anything you had done in the past.

I searched for document_valid

Sorry I meant DOC_VALID which does exactly your suggested code but allows for Geany to add conditions without your code needing to be changed. Thats not to say all plugins use it properly (or even Geany itself).

The important thing was to beware that state can change between callbacks to your plugin, I saw a note that you made a list (of documents I think) global, thats fine, but if you are saving information between callbacks you have to check that the information in that list is valid every time your plugin is entered because any arbitrary changes could be made (documents opening/closing) between callbacks.

Maybe our miscommunication stems from my lack of experience developing Geany or GTK development, but feel free to elaborate.

Or the problem of the description of the plugin. I had in mind "pin" as being locking the document tab to the left side of the notebook tabs, a feature available in several other applications. Your README.md definition of "pin" as adding to a sidebar list clears that up (and is probably simpler to implement).

[andy5995]

Yes, that was what I was asking, describing a feature as "I want it to work like Vscode" is unhelpful if the reader doesn't use Vscode or that feature of Vscode, whereas "I want it to do xyz when I click blah because I have found it useful in Vscode" is of course fine.

Sounds like we're in agreement in that case. ;)

[end getting in ahead]

I was simply saying that I was making the above point as guidance ahead of you doing anything, not that it was a criticism of anything you had done in the past.

I searched for document_valid

Sorry I meant DOC_VALID which does exactly your suggested code but allows for Geany to add conditions without your code needing to be changed. Thats not to say all plugins use it properly (or even Geany itself).

I'll use that macro, thanks for pointing it out, but as I mentioned in geany/geany#3770 , if doc->is_valid is true, doc->file_name may not necessarily be valid, it may be NULL. This happens when the 'document' in front of the user is an 'untitled' document. Although there's no point in trying to "pin" an untitled document, it's better to prevent a segfault if a user accidentally tries to pin it (like I did).

The important thing was to beware that state can change between callbacks to your plugin, I saw a note that you made a list (of documents I think) global, thats fine, but if you are saving information between callbacks you have to check that the information in that list is valid every time your plugin is entered because any arbitrary changes could be made (documents opening/closing) between callbacks.

Ok. I know I antipated that if a user closes a doc on the pin list and clicks the filename from the pin list later, the doc will be re-opened. Which isn't so terrible but it sounds like at some point I should implement checking the list as you said.

Maybe our miscommunication stems from my lack of experience developing Geany or GTK development, but feel free to elaborate.

Or the problem of the description of the plugin. I had in mind "pin" as being locking the document tab to the left side of the notebook tabs, a feature available in several other applications. Your README.md definition of "pin" as adding to a sidebar list clears that up (and is probably simpler to implement).

If you haven't already looked at the video, or had time to glance at my code, I'll inform you now that it has been implemented. ;)