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

Add a new completion API for experimental plugins feature. #5000

Merged
merged 33 commits into from
Dec 10, 2023

Conversation

Mm2PL
Copy link
Collaborator

@Mm2PL Mm2PL commented Dec 3, 2023

Description

I remade it for the new completion system. Currently it only allows tab completion.
Example plugin using this api: https://github.com/Mm2PL/supibot-completion-plugin
I have updated https://github.com/Mm2PL/chatterino-test-plugin/ to also include a completion example

I might refactor out Unrelated: Add error tracking on load to be a separate PR.

Happy 5k
FeelsBirthdayMan

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

@@ -297,6 +302,39 @@ const std::map<QString, std::unique_ptr<Plugin>> &PluginController::plugins()
{
return this->plugins_;
}
std::pair<bool, QStringList> PluginController::updateCustomCompletions(
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'updateCustomCompletions' can be made static [readability-convert-member-functions-to-static]

src/controllers/plugins/PluginController.hpp:54:

-     std::pair<bool, QStringList> updateCustomCompletions(
+     static std::pair<bool, QStringList> updateCustomCompletions(

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

src/controllers/plugins/LuaUtilities.hpp Outdated Show resolved Hide resolved
@@ -186,6 +291,54 @@
return out;
}

// Represents a Lua function on the stack
template <typename ReturnType, typename... Args>
class CallbackFunction
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: class 'CallbackFunction' defines a non-default destructor, a copy assignment operator and a move assignment operator but does not define a copy constructor or a move constructor [cppcoreguidelines-special-member-functions]

class CallbackFunction
      ^

src/controllers/plugins/PluginController.cpp Show resolved Hide resolved
@Mm2PL Mm2PL changed the title Lua completion API implementation attempt 2 Add a new completion API for experimental plugins feature. Dec 7, 2023
@Mm2PL Mm2PL requested a review from pajlada December 7, 2023 15:23
Copy link
Member

@pajlada pajlada left a comment

Choose a reason for hiding this comment

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

Some nitpicks, would like to see an example plugin written in lua that utilizes the API

src/controllers/plugins/LuaUtilities.hpp Outdated Show resolved Hide resolved
src/controllers/plugins/LuaUtilities.hpp Outdated Show resolved Hide resolved
src/controllers/plugins/LuaUtilities.hpp Outdated Show resolved Hide resolved
src/controllers/plugins/LuaUtilities.hpp Outdated Show resolved Hide resolved
src/controllers/plugins/LuaUtilities.hpp Outdated Show resolved Hide resolved
src/controllers/plugins/LuaUtilities.hpp Outdated Show resolved Hide resolved
src/controllers/plugins/PluginController.cpp Show resolved Hide resolved
src/controllers/plugins/PluginController.cpp Outdated Show resolved Hide resolved
src/controllers/plugins/Plugin.hpp Outdated Show resolved Hide resolved
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

@@ -306,6 +313,46 @@ const std::map<QString, std::unique_ptr<Plugin>> &PluginController::plugins()
{
return this->plugins_;
}
std::pair<bool, QStringList> PluginController::updateCustomCompletions(
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'updateCustomCompletions' can be made static [readability-convert-member-functions-to-static]

src/controllers/plugins/PluginController.hpp:55:

-     std::pair<bool, QStringList> updateCustomCompletions(
+     static std::pair<bool, QStringList> updateCustomCompletions(

@pajlada
Copy link
Member

pajlada commented Dec 10, 2023

The current completion example doesn't work as expected i think - is this an issue in the test plugin or in the c++ code?

e.g. when I type /foo and press it'll remove the /foo and start autocomplete barbazA and barbazB
Same when I type /foo with a space - it'll remove the /foo

@pajlada pajlada enabled auto-merge (squash) December 10, 2023 13:25
@pajlada pajlada merged commit fd4cac2 into master Dec 10, 2023
20 checks passed
@pajlada pajlada deleted the feature/lua-completion-api-v2 branch December 10, 2023 13:41
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.

2 participants