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

Added CTYPE Registry system for content extensions by plugins. #96

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kenanb
Copy link

@kenanb kenanb commented Sep 30, 2015

So I created a complete content system layer on top of the original one. It supports:

  • Content types through defcontent macro.
  • Content modifiers (extension points) through defmodifier macro. Content modifiers are actually classes that directly superclass the specified content types when enabled.
  • No restriction in order of content and modifier definitions (doesn't matter if content plugin or the modifier plugin is loaded first. it still works).
  • Dynamic index system.
  • Proper Content/Index relationship.
  • Content/modifier registry.
  • Full backwards compatible interface for content generated in past by users.

Each content type is created using the defcontent interface. All content types are plugins now. Even posts. It is just ensured to be always loaded (even when absent from plugins list) for backwards compat.
(defmacro defcontent (name indexable-p &rest body)
The following defines an indexable content called post:

(defcontent post t
  ((title  :initarg :title  :reader title-of)
   (author :initarg :author :reader author-of)
   (format :initarg :format :reader post-format))
  (:default-initargs :author nil))

The only difference from the old version is the absence of direct-superclass list and the 't' paremeter. Direct superclasses are now dynamically adjusted by the content modifiers. At least content class is always a superclass. The rest are added by modifiers.
The bool value t after the class name defines if the content is indexable or not. Instances of a content type will always be shown in relevant indices when the content is set indexable. They will be hidden from indexes if indexable-p is set to nil.

Each content modifier is created using defmodifier' interface. (defmacro defmodifier (name ctypes direct-superclasses direct-slots &rest options)`

An example modifier named metadata that modifies post, page and shout content types.

(defmodifier metadata (post page shout) ()
  ((keywords :initarg :keywords :reader keywords-of)
   (description :initarg :description :reader description-of)
   (image :initarg :image :reader image-of)
   (card :initarg :card :reader card-format)
   (creator :initarg :creator :reader creator-of))
  (:default-initargs
   :keywords nil
   :description nil
   :image nil
   :card nil
   :creator nil))

A content modifier is defined almost the same way a class is defined except you also add the list of effected content classes immediately after the name.

metadata plugin for testing content modifier plugins.
@kenanb
Copy link
Author

kenanb commented Sep 30, 2015

I also revised metadata plugin according to changes to show how the content/modifier system works.
Posts plugin in plugins folder is an example of content type. Metadata plugin is an example of content modifier.

@kenanb
Copy link
Author

kenanb commented Sep 30, 2015

@kingcons @PuercoPop

My question: Should this supersede/replace the existing twitter-summary-cards plugin or should
they coexist? I assume this is a complete superset of the functionality, is that right?

Yes, it is a complete superset. I didn't even know it existed till halfway though :) I was happy to find a reference when I realized the twitter-summary-cards plugin. It is a complete superset with support for multiple twitter card types and other social media share metadata as well as SEO metadata. It also prevents exceptions like cropped description tag breaking HTML because of odd number of doublequotes inside the cropped description text.

The main issue with the PR is that it modifies the post class instead of extending it.

I happen to agree with this point. Doubling the size of the built-in slots on the post class to accommodate an optional plugin is something I'm not comfortable with. That being said, we do need a real mechanism for plugins that require arbitrary data to be kept with content objects.

I strongly agree with both your points. I did it that way because I thought writing a proper content modifier system would take a long time and the functionality provided by the plugin is too essential to wait. But I got comfortable with the codebase faster than I expected because it was extremely fun working on Coleslaw :D So I decided to write the system that tries to provide all the functionality that I found lacking so far in plugin system. The design actually shaped up while I was trying to figure out how to design shouts content type. I was almost finished with shouts plugin and I got stuck because of the way plugins worked. Especially the lack of formal relation between indices and content blocked me. So here is my proposal. I hope you like it.

I can finish the shouts plugin in 2 days if this PR gets accepted.

@PuercoPop Your ideas about how to write the extension system helped me come up with this design btw. So thank you for discussing the possible architectures. Though instead of creating a mapping between content types and extensions, I designed the system in reverse (if I understood your proposal correctly).

The one-on-one mapping between the content subclasses and file extensions stay exactly as they are. But all content modifier plugins that are enabled dynamically add themselves as superclasses to the content types they effect. So the actual hierarchy of a content class is only defined when a specific blog configuration is loaded.

Each content type is a subclass of the content class, you (normally) do not subclass content subclasses to extend them, instead you add modifier classes to content class to inherit modifiers properties. I think that is kind of more suitable because there wont be a million different types of content that heavily inherit from each other but almost all modifier types will try to extend more than one content type. So far testing with several plugin types that I am writing it feels pretty elegant TBH.

And it didn't really error in any case yet. Of course I didn't test it with the plugins that are not revised for the new architecture. But it takes about 2 minutes to revise an old plugin to the new system really. So I think it will not be a problem.

@kingcons
Copy link
Collaborator

kingcons commented Oct 6, 2015

@kenanb This is interesting and impressive work but it's definitely a more invasive change than the previous PR and I would feel uncomfortable merging it without some more thought. I'd also still really like to get a good test suite in place before more big changes drop in coleslaw but it's tough with teaching and the semester being in full swing.

I like the idea of a defcontent macro and the idea of some kind of central registry is interesting but I feel like a simpler solution is possible wherein (as mentioned before) any initargs not matching the content type are stored as a hash or plist in a :metadata slot which could be added to the content class.

We could add some sugar on to that to make sure that plugins could easily validate or ensure that all needed metadata was present on a CTYPE. Anyway, I'm not comfortable merging this right away but it is excellent food for thought. Thanks!

@kenanb
Copy link
Author

kenanb commented Oct 6, 2015

@kingcons of course a simpler solution is possible for extra metadata. This PR doesn't actually try to fix that problem, it tries to fix the problems you defined in Plugin Constaints and Better Content Types section of hacking.md. The central registry part is not even extremely important, I thought it would be good to have but I am creating another version of content and modifier protocols that works without a registry and instead relies on class allocated slots and MOP.

Anyway, I totally understand you feeling uncomfortable merging it. these are big changes after all. But all plugins I write will rely on the serious changes I make to the plugin system and I really need to add this functionality to make coleslaw usable for the future projects I have. So I will have to concentrate on my fork instead of contributing to this repo. I mean my plugins will be incompatible, at least without some tweaks. I hope that is allright for you.

@kingcons
Copy link
Collaborator

kingcons commented Oct 7, 2015

@kenanb That makes perfect sense. Thanks for your efforts. Hopefully in a little while I can see how your improvements turn out and incorporate them as I'm able. :)

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