samuelet / indexmenu

A dokuwiki plugin to show a customizable and sortable index for a namespace.
http://dokuwiki.org/plugin:indexmenu
GNU General Public License v2.0
47 stars 43 forks source link

Fix deprecations: html_li_index and PageResolver #275

Closed nerun closed 11 months ago

nerun commented 11 months ago

Nothing special to say: html_li_index is deprecated, tagListItem should be used instead. I am receiving a massive deprecation warning about this every day.

Also fixed PageResolver deprecation warning.

Klap-in commented 11 months ago

You changed both function names, while only one is deprecated.

(btw I’m working also on a new version which uses another JavaScript tree. Unfortunately the work on that is a bit slow, but that is the reason this version is delaying. Maybe I should consider to check and release that version, without changing the defaults too much, as more and more people need this kind of fixes)

nerun commented 11 months ago

Thank you. It's now fixed!

Love this index. I use with "gnome" theme, looks great!

[Off topic > Hey, have you seen my new plugin? Please, take a look: DokuWiki - Plugin: ParserFunctions.]

Klap-in commented 11 months ago

The callback named _html_list_index in syntax_plugin_indexmenu_indexmenu is not changed. So I'm wondering why this works, as you are renaming it in the place where it is called. I would guess it does not work?

[offtopic: nice, parserfunctions can be handy for conditional things. Keep it simple ;-) for example indexmenu plugin can a bit too much, which complicates maintenance.]

nerun commented 11 months ago

The callback named _html_list_index in syntax_plugin_indexmenu_indexmenu is not changed. So I'm wondering why this works, as you are renaming it in the place where it is called. I would guess it does not work?

Well, i was receiving the deprecation warning about html_list_index every day, tons a day. And, now nothing.

Another thing very important, for example, it was:

$out_tmp = html_buildlist($data, 'idx', array($idxm, "_html_list_index"), "html_li_index");

html_li_index is deprecated, and _html_list_index IS NOT html_li_index. They are not the same function. Please note the _list_ against _li_ in the middle of the function name.

_html_list_index is an internal function to plugin, while html_li_index is external and is deprecated.

Apparently the last item in the array (html_li_index) tells to _html_list_index to use html_li_index. Updating to tagListItem seems to fix it.

Klap-in commented 11 months ago

_html_list_index is an internal function to plugin, while html_li_index is external and is deprecated.

Exactly, therefore the other two _html_list_index needs to be renamed to the original name

nerun commented 11 months ago

Exactly, therefore the other two _html_list_index needs to be renamed to the original name

HAHAHA I finally got it! Thank you for your patience! Fixed now. But i did a force push to don't pollute the git.

Klap-in commented 11 months ago

Did you check if the _parse_ns() produce the same output as before?

Here an alternative replacement: https://github.com/samuelet/indexmenu/pull/269/commits/a3507a2c1d473e7d8f5a5483a56b389d4fe02116#

nerun commented 11 months ago

Updated but... Do you think is better i split PageResolver in another PR?

Klap-in commented 11 months ago

In general, yes. Bit I will merge as is now. Thanks!