Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overview of currently deprecated symbols #3019

Open
xiota opened this issue Nov 25, 2021 · 6 comments
Open

Overview of currently deprecated symbols #3019

xiota opened this issue Nov 25, 2021 · 6 comments

Comments

@xiota
Copy link
Contributor

xiota commented Nov 25, 2021

In my opinion, deprecated symbols that meet the following criteria would be safe to remove:

  • have been deprecated for more than a few years (pretty much all of them)
  • aren't used by any plugins or used only by "dead" plugins (that no longer compile because of dependency issues).

doc/plugins.dox states that the legacy plugin entry points are deprecated. However, they are not formally marked as deprecated, so should not be candidates for removal. Also, 22 plugins still reference 'void plugin_init': geanyextrasel, geanyinsertnum, geanylua, geanymacro, geanyminiscript, geanynumberedbookmarks, geanypg, geanyprj, geniuspaste, markdown, multiterm, pairtaghighlighter, pretty-printer, projectorganizer, scope, sendmail, shiftcolumn, spellcheck, tableconvert, updatechecker, vimode, xmlsnippets

Here is a list of deprecated symbols that are marked with @deprecated, GEANY_DEPRECATED, GEANY_DEPRECATED_FOR, and GEANY_DISABLE_DEPRECATED. Also noted are the version/API/date of deprecation (if known) and plugins they are used in.

document.h (#3020)

  • documents_foreach(i) (use foreach_document instead)
    • not used by any plugins

filetypes.h (#3020)

  • filetype_id (use GeanyFiletypeID instead)
    • not used by any plugins

keybindings.h (#3029)

  • GEANY_KEYS_EDITOR_MACROLIST (since 1.25)
    • used by geanylua
    • removal would affect values of other symbols in the enum; it could be "removed" by renaming to something else (like _REMOVED_GEANY_KEYS_EDITOR_MACROLIST_) and changing the doxygen comment into a normal one

plugindata.h

  • Remove deprecated symbols from plugindata.h #3072
    • GeanyFunctions
      • not used by any plugins
    • GeanyKeyGroupInfo (use plugin_set_key_group instead)
      • not used by any plugins
    • PLUGIN_KEY_GROUP (use plugin_set_key_group() instead)
      • not used by any plugins
    • PluginFlags (use ui_add_document_sensitive instead)
      • used by multiterm
    • PluginFields (use ui_add_document_sensitive instead)
      • used by multiterm
    • document_reload_file (use document_reload_force instead)
      • used by multiterm
    • DOC_IDX(doc_ptr)
      • not used by any plugins
    • DOC_IDX_VALID(doc_idx)
      • not used by any plugins
    • GEANY_WINDOW_MINIMAL_WIDTH
      • not used by any plugins
    • GEANY_WINDOW_MINIMAL_HEIGHT (use GEANY_DEFAULT_DIALOG_HEIGHT instead)
      • not used by any plugins
  • Remove deprecated symbols: PROXY_IGNORED, PROXY_MATCHED, PROXY_NOLOAD #3030
    • PROXY_IGNORED (since 1.26/226, use GEANY_PROXY_IGNORE instead)
      • used by geanypy
    • PROXY_MATCHED (since 1.26/226, use GEANY_PROXY_MATCH instead)
      • used by geanypy
    • PROXY_NOLOAD (since 1.26/226)
      • not used by any plugin

sciwrappers.h (#3025)

  • sci_get_text (use sci_get_contents instead)
    • used by document.c
    • not used by any plugins (geanygendoc references it in a comment)
  • sci_get_selected_text (use sci_get_selection_contents instead)
    • not used by any plugins (geanyctags references it in a comment)
  • sci_get_text_range (use sci_get_contents_range instead)
    • used in document.c and editor.c
    • not used by any plugins

tagmanager/tm_source_file.c (#3024)

  • tm_get_real_path (since 1.32/235, use utils_get_real_path instead)
    • used by utils.c
    • not used by any plugins

ui_utils.h (#3028)

  • ui_frame_new_with_alignment (since 1.29, use GTK API directly)
    • used by plugins: geanypy, webhelper
  • ui_widget_set_tooltip_text (since 0.21, use gtk_widget_set_tooltip_text instead);
    • not used by any plugins

utils.h

  • NZV(ptr), deprecated 2013/08, use !EMPTY() instead
    • not used by any plugins
  • setptr(ptr, result), deprecated 2011/11/15, use SETPTR() instead.
@elextr
Copy link
Member

elextr commented Nov 25, 2021

Thanks for making this list.

have been deprecated for more than a few years (pretty much all of them)

Yeah, thats why I raised reviewing them.

Comments on a couple below:

The legacy plugin entry points are called "deprecated" in the discussion in the API manual, but the actual functions are not marked deprecated, so will not produce compile messages. Also as you noted many plugins still use them (see what I said elsewhere about transitions from old to new being incomplete :-). So yeah, gotta stay for now at least, but should be marked deprecated.

I have ranted before about killing document_foreach() with fire, happy to see it go. (Not that foreach_document is much better, lower case macros trap, shudder, if you want a loop, C has a loop, stop trying to be another language [end rant])

filetype_id is just a typedef for another type name, so if anything uses it its cheap to fix.

GeanyFunctions IIRC thats from a method of restricting which functions were visible to plugins from before the current method using Geany as a library and GEANY_API_SYMBOL to make functions visible from the library. So should be removable no problems.

GEANY_KEYS_EDITOR_MACROLIST the deprecation says it does nothing, so probably won't be missed, it could be "removed" without changing the ABI by renaming the enumerator to "unused".

DOC_IDX and DOC_IDX_VALID were potentially dangerous IIRC so probably best if they go ASAP.

The PROXY... ones should be historical and no problem.

The use of sci_get_text in document.c is kind of special in that it is appending the whole scintilla buffer to an existing memory block (containing the BOM), but for those one or two uses SSM could be used directly instead.

In any other uses besides document.c a call to sci_get_text_contents() does what the user code should have, a call to sci_get_text_length() and then to sci_get_text() with a buffer of that length +1 so replacement should not be hard, in fact it should save code in plugins :-)

I can't see a use of sci_get_selected_text() in ui_utils.c, the "uses" I see are calls to sci_get_selected_text_length()

For sci_get_text_range() some of the uses in editor.c look dodgy using a fixed length buffer :-( so replacement would be good (so long as we don't forget the free().

tm_real_path() is broken anyway, (see BUGS here) its just lucky it works most of the time, utils.c should have a reasonable version assuming POSIX 2008 semantics for realpath() so its not broken and possibly the Windows alternative if its still required.

ui_utils.h might have more functions that were replaced by real GTK functions.

NZV can die happily. IM(NS)HO EMPTY can also die and be a real function, but thats another story.

setptr is just a name change of case to make it obvious its a nasty dangerous macro, fixing it in users should be just a replaceall.

@eht16
Copy link
Member

eht16 commented Nov 25, 2021

Thanks @xiota for the list, great job!

I'm all for removing as much as we can without breaking the universe.
So, maybe we could just start with removing the easy ones. This could then be reviewed and merged quickly, so we get more testing in master.

I would also say that breaking plugins which are also currently broken for other reasons (no GTK3, n Webkit, GeanyLua in general, ...), don't need to be considered too badly in this regard.

@elextr
Copy link
Member

elextr commented Nov 25, 2021

And Geanypy will not compile on GTK3 so it doesn't matter either.

@elextr
Copy link
Member

elextr commented Nov 25, 2021

Note PRs should only change one or two at a time in case we have to revert.

@xiota
Copy link
Contributor Author

xiota commented Nov 25, 2021

I can't see a use of sci_get_selected_text() in ui_utils.c

I've corrected the list.

setptr is just a name change of case to make it obvious its a nasty dangerous macro, fixing it in users should be just a replaceall.

Opened a PR to replace setptr with SETPTR in plugins: geany/geany-plugins#1142

... broken... GeanyLua in general...

geany/geany-plugins#1123 needs to be reviewed and merged to get GeanyLua building again so the geany-plugins CI builds will start working again.

@xiota
Copy link
Contributor Author

xiota commented Nov 27, 2021

kugel- pushed a commit that referenced this issue Nov 29, 2021
…#3030)

The only known plugin affected is geanypy. But it is currently broken because of GTK2 dependencies.

See #3019
kugel- pushed a commit that referenced this issue May 3, 2022
This PR removes remaining deprecated symbols from plugindata.h. (See #3019 for overview of currently deprecated symbols.)

- GeanyFunctions
- GeanyKeyGroupInfo
- PluginFields
- PluginFlags
- PLUGIN_KEY_GROUP
- document_reload_file
- DOC_IDX(doc_ptr)
- DOC_IDX_VALID(doc_idx)
- GEANY_WINDOW_MINIMAL_HEIGHT
- GEANY_WINDOW_MINIMAL_WIDTH

The multiterm plugin, which currently doesn't compile because of GTK2-related dependencies, refers to the following symbols: PluginFlags, PluginFields, document_reload_file

The other symbols removed in this PR are not used by any known plugins.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants