To all esteemed developers Frank Lanitz, Matthew Brush. Kindly review my code which i have modified according to your suggestions.
Am 27.02.2013 00:08, schrieb Mayank Jha:
To all esteemed developers Frank Lanitz, Matthew Brush. Kindly review my code which i have modified according to your suggestions.
Shall we guess what your plugin does?
Regarding the embedded python script. I don't think you actually need to write it out to a file, you should be able to pass it to the interpreter from memory (see [1]).
What's the status of the python plugin?
Best regards
On 27 February 2013 10:26, Thomas Martitz thomas.martitz@student.htw-berlin.de wrote:
Am 27.02.2013 00:08, schrieb Mayank Jha:
To all esteemed developers Frank Lanitz, Matthew Brush. Kindly review my code which i have modified according to your suggestions.
Shall we guess what your plugin does?
Regarding the embedded python script. I don't think you actually need to write it out to a file, you should be able to pass it to the interpreter from memory (see [1]).
Plugins should not embed Python since it can only be done once, if two plugins both do it it fails. There have already been problems with trying to use two plugins which both embed python. On the basis that the only likely solution is having a shared Python, nobody should embed Python in their plugins until Geanypy is sorted.
What's the status of the python plugin?
ping codebrainz :)
Best regards
Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
On 13-02-26 03:43 PM, Lex Trotman wrote:
On 27 February 2013 10:26, Thomas Martitz thomas.martitz@student.htw-berlin.de wrote:
Am 27.02.2013 00:08, schrieb Mayank Jha:
To all esteemed developers Frank Lanitz, Matthew Brush. Kindly review my code which i have modified according to your suggestions.
Shall we guess what your plugin does?
Regarding the embedded python script. I don't think you actually need to write it out to a file, you should be able to pass it to the interpreter from memory (see [1]).
Yep, much better way to do it, but for reasons mentioned below.
Plugins should not embed Python since it can only be done once, if two plugins both do it it fails. There have already been problems with trying to use two plugins which both embed python. On the basis that the only likely solution is having a shared Python, nobody should embed Python in their plugins until Geanypy is sorted.
+1
What's the status of the python plugin?
ping codebrainz :)
Except for needed work on sharing keybindings between the Python plugins, it's pretty stable in my experience. Only big thing is to get it integrated into the GP build systems (and as usual API docs[1] are incomplete).
<rant> I still really wish I could keep the code synced in it's own repo on Github, but due to restrictions on Git submodules in GP and the "proprietary" build system, it seems like I'll have to stop being able to provide stand-alone distribution (and way better Github Issue tracker and such). Suggestions would be very welcome if there's a way I could do this without double-maintaining repos, I'm neither a Git nor Autotools expert. </rant>
Cheers, Matthew Brush
1) Please get your indentation right, it makes it hard to read with randomly incorrect indentation, limit lines to ~80 characters, sloppy styling makes it hard to read and does not give us much confidence in the quality of your code, layout the python script string so it is legible, remove the comment its useless. 2) Do not use fixed length text arrays, that has been mentioned before 3) do not g_free malloced memory 4) do not pclose NULL, check the return value 5) don't create and leave junk y.py in the config/plugins directory
Cheers Lex
On 27 February 2013 10:08, Mayank Jha mayank25080562@gmail.com wrote:
To all esteemed developers Frank Lanitz, Matthew Brush. Kindly review my code which i have modified according to your suggestions.
Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Am 27.02.2013 00:08, schrieb Mayank Jha:
To all esteemed developers Frank Lanitz, Matthew Brush. Kindly review my code which i have modified according to your suggestions.
My note without looking into it to much: - README is missing - Licence is missing - Make usage of PLUGIN_SET_TRANSLATABLE_INFO() in combination mit #include "config.h" (check other plugins for it if needed) - Provide a proper patch against geany-plugins including all files - Do you really need to use fprintf() so excessive starting from line 239 as it appears its only static content - Please check formatting of your code http://geany.org/manual/dev/hacking.html should also apply to plugins in most cases - Use C90 compatible code - switch in l96 is not having a default case
Cheers, Frank
thanx a lot for ur feedback. I could not get a few things clearly: 1. Is this readme not OK? https://github.com/mjnovice/geany-plugins/blob/master/codesubmit/README 2. " #include "config.h" (check other plugins for it if needed)" couldn't get what do we need to do here? 3. Do I need to provide a separate text file which contains the licensing, won't the comment in the codesubmit.c do the job? 4. "Provide a proper patch against geany-plugins including all files" how to do this?
On 13-02-28 01:12 PM, Mayank Jha wrote:
thanx a lot for ur feedback. I could not get a few things clearly:
- Is this readme not OK?
https://github.com/mjnovice/geany-plugins/blob/master/codesubmit/README
You need to compile it with doc-utils/rst2html to be sure. Also you should copy other plugins to know what is commonly put in there.
- " #include "config.h" (check other plugins for it if needed)" couldn't
get what do we need to do here?
If you have the plugin inside Geany-Plugins source tree, it shouldn't be a problem. As above, best is to copy other plugins to see how they do it.
- Do I need to provide a separate text file which contains the licensing,
won't the comment in the codesubmit.c do the job?
You probably should, since other plugins do (see the pattern? :).
- "Provide a proper patch against geany-plugins including all files" how
to do this?
Basically the exact opposite of your last pull request :)
It means that your pull request should meet all of the criteria mentioned in your last pull request's comments, the mailing list responses, the HACKING file and those that are implied from fitting in with other plugins. The Pull Request should contain a complete integration of your plugin, including all required files, no unnecessary files, updates to both build systems to make it compile, and no obvious bugs, crashers, leaks, hardcoded paths, embedded Python source code, improper indentation, etc. If you make commit messages like "blah blah" again, I can almost guarantee your pull request will get closed immediately :)
Here are some pretty good examples of integrating a new plugin:
https://github.com/geany/geany-plugins/commit/6c93e9513935820e7919e24722d622...
https://github.com/kugel-/geany-plugins/commit/8eb52e74d399736152030d5b9ef0e...
Cheers, Matthew Brush
Fixed the typo Furter Improvements and next steps