Skip to content

Support configurable extra plugin lookup path#2693

Merged
Frenzie merged 5 commits intokoreader:masterfrom
houqp:extra_plugin_path
Apr 6, 2017
Merged

Support configurable extra plugin lookup path#2693
Frenzie merged 5 commits intokoreader:masterfrom
houqp:extra_plugin_path

Conversation

@houqp
Copy link
Copy Markdown
Member

@houqp houqp commented Mar 29, 2017

This feature can be handy for platforms that does custom packaging like android, where plugins directory is not accessible from users and wiped every time the package is updated.

"highlight_options",
"change_font",
"hyphenation",
"read_timer",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason to prefer this plugin to be placed in tools? IMO, it's more like a "reading experience".

Copy link
Copy Markdown
Member

@Frenzie Frenzie Mar 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does seem a touch out of place with the other rendering options. There should at least be a separator in between if it goes in the typeset menu. I didn't want to make things controversial by changing the menu around (and I didn't have time to think about it anyway) but it should be something more like

        -- rendering stuff
        "page_overlap",
        "switch_zoom_mode",
        "set_render_style",
        --sep, I don't think highlight options go with anything else really
        "highlight_options",
        --sep, font-related (typeset) stuff
        "change_font",
        "floating_punctuation",
        "hyphenation",
        --sep, "reading experience" stuff
        "read_timer"
        --speed reading?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Firstly, for consistent reason. It's placed under tools tab in filemanager. We should either keep it consistent between apps or only enable it in reader.

Secondly, that tab is named typeset, which means anything that controls rendering results. A timer doesn't fit into this category. With the new UX proposed by @baskerville and the new separator feature, we can combine navigation and typeset into one read menu, then it makes more sense to put timer there.

logger.warn("Error when loading", mainfile, plugin_module)
elseif type(plugin_module.disabled) ~= "boolean" or not plugin_module.disabled then
local lookup_path_list = { DEFAULT_PLUGIN_PATH }
local extra_paths = G_reader_settings:readSetting("extra_plugin_paths")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we directly add DataStorage:getDataDir()/plugins/ to this list.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually don't really expect many people to use this feature right now. We can always add a default path to the list if it turns out to be a popular feature. My preference is to not introduce this extra lookup overhead to all users for now.

houqp added 4 commits April 2, 2017 15:16
To keep it consistent between reader and filemanager
…ories

Extra plugin lookup paths can be set in global reader setting via key
extra_plugin_paths. Value of the key can either be a string or an array
of strings.
@houqp houqp force-pushed the extra_plugin_path branch from 8e7525e to 0bc825a Compare April 2, 2017 22:17
@Hzj-jie
Copy link
Copy Markdown
Contributor

Hzj-jie commented Apr 5, 2017

Looks good to me.

@Frenzie Frenzie merged commit 1461574 into koreader:master Apr 6, 2017
@houqp houqp deleted the extra_plugin_path branch April 14, 2017 18:10
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

Successfully merging this pull request may close these issues.

3 participants