Re: [Github-comments] [geany/geany] keybindings: Added missing "Strip Trailing Spaces" (#1806)
Thanks. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1806#issuecomment-405150192
Re: [Github-comments] [geany/geany] keybindings: Added missing "Strip Trailing Spaces" (#1806)
Merged #1806. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1806#event-1734360209
Re: [Github-comments] [geany/geany] keybindings: Added missing "Strip Trailing Spaces" (#1806)
@b4n: I increased the API version again to resolve conflicts. It would be nice if this could be merged before the API gets increased elsewhere again. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1806#issuecomment-405115723
Re: [Github-comments] [geany/geany] keybindings: Added missing "Strip Trailing Spaces" (#1806)
codebrainz commented on this pull request. > @@ -274,6 +274,8 @@ enum GeanyKeyBindingID GEANY_KEYS_FORMAT_SENDTOCMD8, /**< Keybinding. */ GEANY_KEYS_FORMAT_SENDTOCMD9, /**< Keybinding. */ GEANY_KEYS_EDITOR_DELETELINETOBEGINNING,/**< Keybinding. */ + GEANY_KEYS_DOCUMENT_STRIPTRAILINGSPACES,/**< Keybinding. + * @since 1.34 */ If you guys want I can try and update the idea in that commit to work with the contemporary plugin API. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1806#discussion_r179865626
Re: [Github-comments] [geany/geany] keybindings: Added missing "Strip Trailing Spaces" (#1806)
LarsGit223 commented on this pull request. > @@ -59,7 +59,7 @@ G_BEGIN_DECLS * @warning You should not test for values below 200 as previously * @c GEANY_API_VERSION was defined as an enum value, not a macro. */ -#define GEANY_API_VERSION 235 +#define GEANY_API_VERSION 236 So I go up to 237? Or revert the change? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1806#discussion_r179840562
Re: [Github-comments] [geany/geany] keybindings: Added missing "Strip Trailing Spaces" (#1806)
b4n commented on this pull request. > @@ -59,7 +59,7 @@ G_BEGIN_DECLS * @warning You should not test for values below 200 as previously * @c GEANY_API_VERSION was defined as an enum value, not a macro. */ -#define GEANY_API_VERSION 235 +#define GEANY_API_VERSION 236 This is the version currently in master as of bec3832359cf3a7d9f9d5d0a9ac3a622d6387d66, so changing it here is not very useful and likely to lead to conflicts or confusion. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1806#pullrequestreview-110158049
Re: [Github-comments] [geany/geany] keybindings: Added missing "Strip Trailing Spaces" (#1806)
@elextr: sorry for not checking HACKING (in my mind I said **RTFM** to myself :wink: ) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1806#issuecomment-379293444
Re: [Github-comments] [geany/geany] keybindings: Added missing "Strip Trailing Spaces" (#1806)
@elextr: seems to look correct:  -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1806#issuecomment-379292510
Re: [Github-comments] [geany/geany] keybindings: Added missing "Strip Trailing Spaces" (#1806)
b4n commented on this pull request. > @@ -274,6 +274,8 @@ enum GeanyKeyBindingID GEANY_KEYS_FORMAT_SENDTOCMD8, /**< Keybinding. */ GEANY_KEYS_FORMAT_SENDTOCMD9, /**< Keybinding. */ GEANY_KEYS_EDITOR_DELETELINETOBEGINNING,/**< Keybinding. */ + GEANY_KEYS_DOCUMENT_STRIPTRAILINGSPACES,/**< Keybinding. + * @since 1.34 */ @codebrainz right now I kind of like your proposal. Yeah it's less specific, but avoids neverending noise of updating API version at each API-facing change. I'd vote for that :) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1806#discussion_r179768829
Re: [Github-comments] [geany/geany] keybindings: Added missing "Strip Trailing Spaces" (#1806)
@kugel- thats what I suspected, but thought maybe it would still work if its all part of the same comment? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1806#issuecomment-379154587
Re: [Github-comments] [geany/geany] keybindings: Added missing "Strip Trailing Spaces" (#1806)
@LarsGit223 from HACKING ``` * Pass the *--enable-api-docs* option to ``configure``. * Run ``make`` from the doc/ subdirectory. ``` -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1806#issuecomment-379154250
Re: [Github-comments] [geany/geany] keybindings: Added missing "Strip Trailing Spaces" (#1806)
I think the trailing doc comment supports only a brief description (not detail), such that enums/struct members can still be one-liners. But it may be possible to add commands like @since to that, have to try (but I'd expect it should be on the same line still). In any event, you can make full-fledged multi-line doxygen comments for members if you put it before, see [struct GeanyData](https://github.com/geany/geany/blob/master/src/plugindata.h#L170) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1806#issuecomment-379152846
Re: [Github-comments] [geany/geany] keybindings: Added missing "Strip Trailing Spaces" (#1806)
@elextr: no I didn't. That's doxygen building the API docs right? How do I run it on build or is it started on every build? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1806#issuecomment-379152775
Re: [Github-comments] [geany/geany] keybindings: Added missing "Strip Trailing Spaces" (#1806)
@LarsGit223 just a question, did you look at the generated API docs to check that putting the @since on another line looked alright in the output? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1806#issuecomment-379151860
Re: [Github-comments] [geany/geany] keybindings: Added missing "Strip Trailing Spaces" (#1806)
LarsGit223 commented on this pull request. > @@ -274,6 +274,8 @@ enum GeanyKeyBindingID GEANY_KEYS_FORMAT_SENDTOCMD8, /**< Keybinding. */ GEANY_KEYS_FORMAT_SENDTOCMD9, /**< Keybinding. */ GEANY_KEYS_EDITOR_DELETELINETOBEGINNING,/**< Keybinding. */ + GEANY_KEYS_DOCUMENT_STRIPTRAILINGSPACES,/**< Keybinding. + * @since 1.34 */ @b4n @elextr @codebrainz: done and squashed together. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1806#discussion_r179662868
Re: [Github-comments] [geany/geany] keybindings: Added missing "Strip Trailing Spaces" (#1806)
elextr commented on this pull request. > @@ -274,6 +274,8 @@ enum GeanyKeyBindingID GEANY_KEYS_FORMAT_SENDTOCMD8, /**< Keybinding. */ GEANY_KEYS_FORMAT_SENDTOCMD9, /**< Keybinding. */ GEANY_KEYS_EDITOR_DELETELINETOBEGINNING,/**< Keybinding. */ + GEANY_KEYS_DOCUMENT_STRIPTRAILINGSPACES,/**< Keybinding. + * @since 1.34 */ No problem with `@since` being both, lets agree on that and let this be fixed and committed. > IIRC there was an objection because of the theoretical case where some > developer might want to use intermediate API versions in-between release > without having to keep their feature branch up-to-date or something. As I said above, thats less of a likely issue with the faster release cycle. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1806#discussion_r179662429
Re: [Github-comments] [geany/geany] keybindings: Added missing "Strip Trailing Spaces" (#1806)
@LarsGit223 pushed 1 commit. 39ec770 keybindings: adjusted '@since' comment, increased API version -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/1806/files/e7937ffcca8d13506a28dd382a73500e8d4cb2b9..39ec770549821bb8f937921c1e1ad4839f2c477b
Re: [Github-comments] [geany/geany] keybindings: Added missing "Strip Trailing Spaces" (#1806)
codebrainz commented on this pull request. > @@ -274,6 +274,8 @@ enum GeanyKeyBindingID GEANY_KEYS_FORMAT_SENDTOCMD8, /**< Keybinding. */ GEANY_KEYS_FORMAT_SENDTOCMD9, /**< Keybinding. */ GEANY_KEYS_EDITOR_DELETELINETOBEGINNING,/**< Keybinding. */ + GEANY_KEYS_DOCUMENT_STRIPTRAILINGSPACES,/**< Keybinding. + * @since 1.34 */ I like the current practice of putting both the Geany version and the API version since as a plugin developer you're forced to know both and not have to reverse engineer one from the other is useful (which is why this was started IIRC). You need to know the Geany version to know which releases of Geany the plugin is actually compatible with, but you also need to know the API version to satisfy the `plugin_version_check()` thing in the plugin API. In a perfect world we'd just get rid of the plugin API version and use only the release version to make the whole concept simpler. I have [an old commit](https://github.com/codebrainz/geany/commit/2a678cf0513fe7fd5163708c586adf5c545ed589) that added this in a backwards compatible way. IIRC there was an objection because of the theoretical case where some developer might want to use intermediate API versions in-between release without having to keep their feature branch up-to-date or something. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1806#discussion_r179649195
Re: [Github-comments] [geany/geany] keybindings: Added missing "Strip Trailing Spaces" (#1806)
elextr commented on this pull request. > @@ -274,6 +274,8 @@ enum GeanyKeyBindingID GEANY_KEYS_FORMAT_SENDTOCMD8, /**< Keybinding. */ GEANY_KEYS_FORMAT_SENDTOCMD9, /**< Keybinding. */ GEANY_KEYS_EDITOR_DELETELINETOBEGINNING,/**< Keybinding. */ + GEANY_KEYS_DOCUMENT_STRIPTRAILINGSPACES,/**< Keybinding. + * @since 1.34 */ Well, now @b4n has brought it up, yes this should increment the API, but probably not the ABI since although GEANY_KEYS_COUNT will change it says "must not be used in plugins". And IMO the `@since` should be the API, thats what plugin writers care about and need to specify to ensure a compatible Geany version. Theoretically the API could change several times in a release cycle, though that was more of a problem back when it was years (literally) between releases. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1806#discussion_r179616956
Re: [Github-comments] [geany/geany] keybindings: Added missing "Strip Trailing Spaces" (#1806)
Apart from the `@since` bikeshed, LGTM (tested working) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1806#issuecomment-379066503
Re: [Github-comments] [geany/geany] keybindings: Added missing "Strip Trailing Spaces" (#1806)
b4n commented on this pull request. > @@ -274,6 +274,8 @@ enum GeanyKeyBindingID GEANY_KEYS_FORMAT_SENDTOCMD8, /**< Keybinding. */ GEANY_KEYS_FORMAT_SENDTOCMD9, /**< Keybinding. */ GEANY_KEYS_EDITOR_DELETELINETOBEGINNING,/**< Keybinding. */ + GEANY_KEYS_DOCUMENT_STRIPTRAILINGSPACES,/**< Keybinding. + * @since 1.34 */ @LarsGit223 yes. Though as I just updated it, maybe some people would argue that we could re-use this… which adds less noise but is less precise. Opinions @elextr @codebrainz? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1806#discussion_r179584203
Re: [Github-comments] [geany/geany] keybindings: Added missing "Strip Trailing Spaces" (#1806)
LarsGit223 commented on this pull request. > @@ -274,6 +274,8 @@ enum GeanyKeyBindingID GEANY_KEYS_FORMAT_SENDTOCMD8, /**< Keybinding. */ GEANY_KEYS_FORMAT_SENDTOCMD9, /**< Keybinding. */ GEANY_KEYS_EDITOR_DELETELINETOBEGINNING,/**< Keybinding. */ + GEANY_KEYS_DOCUMENT_STRIPTRAILINGSPACES,/**< Keybinding. + * @since 1.34 */ So should I increase the API version also (and add it to the since-comment)? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1806#discussion_r179582778
Re: [Github-comments] [geany/geany] keybindings: Added missing "Strip Trailing Spaces" (#1806)
b4n commented on this pull request. > @@ -274,6 +274,8 @@ enum GeanyKeyBindingID GEANY_KEYS_FORMAT_SENDTOCMD8, /**< Keybinding. */ GEANY_KEYS_FORMAT_SENDTOCMD9, /**< Keybinding. */ GEANY_KEYS_EDITOR_DELETELINETOBEGINNING,/**< Keybinding. */ + GEANY_KEYS_DOCUMENT_STRIPTRAILINGSPACES,/**< Keybinding. + * @since 1.34 */ Well, actually both, sorry: `@since 1.34 (API 23X)` is the best. But yours was historically OK, so maybe it's fine -- and I can change this myself when merging anyway. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1806#discussion_r179578904
Re: [Github-comments] [geany/geany] keybindings: Added missing "Strip Trailing Spaces" (#1806)
b4n commented on this pull request. > @@ -274,6 +274,8 @@ enum GeanyKeyBindingID GEANY_KEYS_FORMAT_SENDTOCMD8, /**< Keybinding. */ GEANY_KEYS_FORMAT_SENDTOCMD9, /**< Keybinding. */ GEANY_KEYS_EDITOR_DELETELINETOBEGINNING,/**< Keybinding. */ + GEANY_KEYS_DOCUMENT_STRIPTRAILINGSPACES,/**< Keybinding. + * @since 1.34 */ Actually it should be a `GEANY_API_VERSION` number -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1806#pullrequestreview-109848185
Re: [Github-comments] [geany/geany] keybindings: Added missing "Strip Trailing Spaces" (#1806)
Friendly ping. It would be cool if this simple fix could be merged at the next opportunity. So we would make sure it's in 1.34, have another issue closed, and I could remove my branch :smile: -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1806#issuecomment-379044814
Re: [Github-comments] [geany/geany] keybindings: Added missing "Strip Trailing Spaces" (#1806)
Done, added the ```@since``` tag. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1806#issuecomment-373460604
Re: [Github-comments] [geany/geany] keybindings: Added missing "Strip Trailing Spaces" (#1806)
b4n commented on this pull request. LGBI, but for the doc comment > @@ -274,6 +274,7 @@ enum GeanyKeyBindingID GEANY_KEYS_FORMAT_SENDTOCMD8, /**< Keybinding. */ GEANY_KEYS_FORMAT_SENDTOCMD9, /**< Keybinding. */ GEANY_KEYS_EDITOR_DELETELINETOBEGINNING,/**< Keybinding. */ + GEANY_KEYS_DOCUMENT_STRIPTRAILINGSPACES,/**< Keybinding. */ We should have an `@since` doc tag for this, as it's part of the API -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1806#pullrequestreview-104193863
Re: [Github-comments] [geany/geany] keybindings: Added missing "Strip Trailing Spaces" (#1806)
LGBI -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1806#issuecomment-373197996
