progval / Supybot-plugins

Collection of plugins for Supybot/Limnoria I wrote or forked.
https://github.com/ProgVal/Limnoria/
107 stars 63 forks source link

Modified SupyML to be much more powerful #345

Closed CorvusCorax closed 3 years ago

CorvusCorax commented 3 years ago

The modification is (mostly)backwards compatible. Existing scripts should still be working (all existing test cases remain fully functional)

Added functionality:

  1. If statement (implemented as expression...
  2. For loop (implemented as integer...
  3. ForEach loop (implemented as string string... string...
  4. Accessing current iterator/token in foreach via within the loop (read only)
  5. Support for IRC commands in nested statements nicks...
  6. Added documentation and examples

Removed functionality:

  1. Variables are no longer global but valid only within the current SupyML script (Reason: Global Variables had no User specific scope and could be accessed by anyone. Scripts run by one user could affect scripts by others with the same variables, with unpredictable outcome. Memory leaks, ..., ..., ...) Suggested workaround: Use existing persistent storage capabilities such as config values, attributes or 3rd party modules to store information permanently.
CorvusCorax commented 3 years ago

this actually addresses issue #56 :)

CorvusCorax commented 3 years ago

the exception handling code I added addresses issue #285 now that SupyML has loops, conditionals, and exception handling (and indirectly through aka's also functions) it's becoming a quite useful and powerful language ;)

One thing is, config.commands.nested.maximum needs to be increased for SupyML to be useful, as the latest code correctly increases nesting with each nested xml command.

progval commented 3 years ago

Why is ErrorReportingProxy declared inside a method? You should be able to declare it outside, and make the constructor take the parser as parameter

CorvusCorax commented 3 years ago

Why is ErrorReportingProxy declared inside a method? You should be able to declare it outside, and make the constructor take the parser as parameter

ErrorReportingProxy is a derived class of whatever SupyML.Proxy might be (accessed through self._plugin.Proxy) in practice this is almost always "<class 'supybot.callbacks.NestedCommandsIrcProxy'>" but I am not sure this is guaranteed and future proof, as it could also end up being a derived class, so hardcoding that in a class definition outside of this property's scope might not be a good idea.

Besides, the Limnoria inbuilt "Conditional" plugin does it just like that - also with an inside-method-scope class definition (in fact I mostly copy-pasted from there) so I figured this must be good practice for this particular use-case:

https://github.com/progval/Limnoria/blob/068488c546612ee0198cecf1a4a46e2667551bcf/plugins/Conditional/plugin.py#L302

note: It should be noted that in the Conditional plugin, the call to the ErrorReportingProxy's constructor is in a try: catch block, with an ignored exception of ArgumentError. I skipped that, because NestedCommandsIrcProxy does not expose this kind of exception (or any exception) because of its own set of handlers in finalEval, where reply()'s are generated based on these errors. This is the whole reason I had to implement this ErrorReportingDummyClass

One could in theory place the ErrorReportingDummyClass definition into global scope and pass both the parser, the ErrorReportingProxy and the callback object in the constructor, but I'm not sure if this would actually be good practice. I see no benefits but it increases complexity and decreases readability.

progval commented 3 years ago

but I am not sure this is guaranteed and future proof

It is. If it ever needs to change, we'll add an alias so it still works for existing plugins.

Besides, the Limnoria inbuilt "Conditional" plugin does it just like that

Ah, alright then.