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

context menu for python plugins #1366

Closed
wants to merge 3 commits into from
Closed

Conversation

medoni
Copy link

@medoni medoni commented Apr 11, 2017

Added context menu for python plugins.

I also updated the HelloWorld python example. The data parameter in "context_menu" is the passed context data.

This change also affects ExecutablePlugin.
master...medoni:master#diff-ab61c87f81bd4fdb87cf6e8a96d6a696
The change in ExecutablePlugin was not tested. (missing example)

Copy link
Member

@bao-qian bao-qian left a comment

Choose a reason for hiding this comment

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

could you clean the git diff for Wox.Core/Plugin/JsonRPCPlugin.cs?
it's hard to see what you changed in that file.

@@ -36,7 +36,7 @@ function Copy-Resources ($path, $config) {
$target = "$output\$config"
Copy-Item -Recurse -Force $project\Themes\* $target\Themes\
Copy-Item -Recurse -Force $project\Images\* $target\Images\
Copy-Item -Recurse -Force $path\Plugins\HelloWorldPython $target\Plugins\HelloWorldPython
Copy-Item -Recurse -Force $path\Plugins\HelloWorldPython\* $target\Plugins\HelloWorldPython
Copy link
Member

Choose a reason for hiding this comment

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

why * since it is recursive ?

Copy link
Author

Choose a reason for hiding this comment

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

My output of HelloWorldPython was wrong. I had a extra HelloWorldPython in the path. My output was

Output\Debug\Plugins\HelloWorldPython\HelloWorldPython\*

Currently I can't reproduce it. Please skip the change

Copy link
Member

Choose a reason for hiding this comment

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

You need one more commit to remove this change, I can't skip it.

@bao-qian
Copy link
Member

overall this is a great PR! just a few bits needs to be clarify.

@medoni
Copy link
Author

medoni commented Apr 11, 2017

 Wox.Core/Plugin/JsonPRCModel.cs | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Wox.Core/Plugin/JsonPRCModel.cs b/Wox.Core/Plugin/JsonPRCModel.cs
index e513f3d..7bcac4c 100644
--- a/Wox.Core/Plugin/JsonPRCModel.cs
+++ b/Wox.Core/Plugin/JsonPRCModel.cs
@@ -71,7 +71,9 @@ namespace Wox.Core.Plugin
 
         private string GetParamterByType(object paramter)
         {
-
+            if (paramter == null) {
+                return "null";
+            }
             if (paramter is string)
             {
                 return string.Format(@"\""{0}\""", RepalceEscapes(paramter.ToString()));

The other changes in JsonPRCModel.cs are miss spelling of the name parameter and replace.

d8eca09#diff-f1a703a9da7e618aa28157daa4e22fd2R72
d8eca09#diff-f1a703a9da7e618aa28157daa4e22fd2R92

@bao-qian
Copy link
Member

Yes a i know .
But i want to understand about Wox.Core/Plugin/JsonRPCPlugin.cs

@medoni
Copy link
Author

medoni commented Apr 11, 2017

sry, wrong file.

Here is the diff without whitespace changes.

 Wox.Core/Plugin/JsonRPCPlugin.cs | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/Wox.Core/Plugin/JsonRPCPlugin.cs b/Wox.Core/Plugin/JsonRPCPlugin.cs
index 2855fa1..369bd16 100644
--- a/Wox.Core/Plugin/JsonRPCPlugin.cs
+++ b/Wox.Core/Plugin/JsonRPCPlugin.cs
@@ -17,7 +17,7 @@ namespace Wox.Core.Plugin
     /// Represent the plugin that using JsonPRC
     /// every JsonRPC plugin should has its own plugin instance
     /// </summary>
-    internal abstract class JsonRPCPlugin : IPlugin
+    internal abstract class JsonRPCPlugin : IPlugin, IContextMenu
     {
         protected PluginInitContext context;
         public const string JsonRPC = "JsonRPC";
@@ -29,14 +29,33 @@ namespace Wox.Core.Plugin
 
         protected abstract string ExecuteQuery(Query query);
         protected abstract string ExecuteCallback(JsonRPCRequestModel rpcRequest);
+        protected abstract string ExecuteContextMenu(Result selectedResult);
 
         public List<Result> Query(Query query)
         {
             string output = ExecuteQuery(query);
-            if (!String.IsNullOrEmpty(output))
-            {
-                try
-                {
+            try {
+                return DeserializeResult(output);
+            }
+            catch (Exception e) {
+                Log.Exception($"|JsonRPCPlugin.Query|Exception when query <{query}>", e);
+                return null;
+            }
+        }
+
+        public List<Result> LoadContextMenus(Result selectedResult) {
+            string output = ExecuteContextMenu(selectedResult);
+            try {
+                return DeserializeResult(output);
+            }
+            catch (Exception e) {
+                Log.Exception($"|JsonRPCPlugin.LoadContextMenus|Exception on result <{selectedResult}>", e);
+                return null;
+            }
+        }
+
+        private List<Result> DeserializeResult(string output) {
+            if (!String.IsNullOrEmpty(output)) {
                 List<Result> results = new List<Result>();
 
                 JsonRPCQueryResponseModel queryResponseModel = JsonConvert.DeserializeObject<JsonRPCQueryResponseModel>(output);
@@ -72,14 +91,10 @@ namespace Wox.Core.Plugin
                     results.Add(result);
                 }
                 return results;
-                }
-                catch (Exception e)
-                {
-                    Log.Exception($"|JsonRPCPlugin.Query|Exception when query <{query}>", e);
-                }
-            }
+            } else {
                 return null;
             }
+        }
 
         private void ExecuteWoxAPI(string method, object[] parameters)
         {

There is a new List<Result> DeserializeResult(string output) function to create Result instances which are used by Query and LoadContextMenu.

        protected abstract string ExecuteContextMenu(Result selectedResult);

        public List<Result> Query(Query query)
        {
            string output = ExecuteQuery(query);
            try {
                return DeserializeResult(output);
            }
            catch (Exception e) {
                Log.Exception($"|JsonRPCPlugin.Query|Exception when query <{query}>", e);
                return null;
            }
        }

        public List<Result> LoadContextMenus(Result selectedResult) {
            string output = ExecuteContextMenu(selectedResult);
            try {
                return DeserializeResult(output);
            }
            catch (Exception e) {
                Log.Exception($"|JsonRPCPlugin.LoadContextMenus|Exception on result <{selectedResult}>", e);
                return null;
            }
        }

        private List<Result> DeserializeResult(string output) {
            if (!String.IsNullOrEmpty(output)) {
                List<Result> results = new List<Result>();

                JsonRPCQueryResponseModel queryResponseModel = JsonConvert.DeserializeObject<JsonRPCQueryResponseModel>(output);
                if (queryResponseModel.Result == null) return null;

bao-qian pushed a commit that referenced this pull request Apr 11, 2017
bao-qian pushed a commit that referenced this pull request Apr 11, 2017
bao-qian pushed a commit that referenced this pull request Apr 11, 2017
bao-qian pushed a commit that referenced this pull request Apr 11, 2017
@bao-qian
Copy link
Member

bao-qian commented Apr 11, 2017

I cherry-pick the commits for you.
please take a look 77e818b fbc8ac1 f323756 be8261a

because these are important files, I need keep the git log clean.

@bao-qian bao-qian closed this Apr 11, 2017
bao-qian pushed a commit that referenced this pull request Dec 19, 2017
bao-qian pushed a commit that referenced this pull request Dec 19, 2017
bao-qian pushed a commit that referenced this pull request Dec 19, 2017
bao-qian pushed a commit that referenced this pull request Dec 19, 2017
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