Pull requests

#2 Merged
Repository
delfick delfick
Branch
default
Repository
fabianbuechler fabianbuechler
Branch
default

Some improvements

Author
  1. Stephen Moore avatarStephen Moore
Reviewers
Description

Hi there,

I can create seperate branches and do three seperate pull request for these features if you want, but there's only three commits, so I'll put them all in here for now :p

I wanted your library to do some things it didn't already:

  1. Better check for feincms in the javascript

    My project doesn't have feincms and so the way you check if it exists fails when it hasn't been defined yet.

  2. Make it possible to specify import paths to markup types in my settings.py

    It would seem when I import markupmirror in my settings.py to register a markup type there, your code imports my settings and it all fails. So I've made it so that you can specify import paths that are resolved by the markup pool.

    As part of this change it was necessary to make the conditional registration a bit more declarative and so I added a "requires" functionality to markup types where they say what libraries are required and markup pool will try to import them and only register the markup type if all the requirements could be fullfilled.

  3. I wanted to access some extra POST variables when doing my conversion which meant access to the request object.

    This change is questionable. I've made it so that __call__ for the markup type will be given a request object by the preview view and it will pass request onto convert, before_convert and after_convert, only if they take a request parameter.

:)

Update:: I've added another couple commits that does a similar thing to what I did with passing the request onto the converters so that I can get the instance of the model being saved when a markup is being called because we saved a model.

Comments (6)

  1. Fabian Büchler repo owner

    Hi Stephen,

    thanks for your effort! I'll look into your request(s) in detail as soon as time allows. (might take until the weekend before I actually get to do it)

    Regards, Fabian

  2. Fabian Büchler repo owner

    Stephen Moore, I just checked your pull request now, and got some quesions / feedback:

    1. FeinCMS check is great.

    2. I agree with your changes, but I'll make some small changes to your code. That will take a while, but hopefully not too long.

    3. Can you explain, why you need access to the request during text conversion? Wouldn't it be possible to register custom converters (e.g. multiple different Markdown converters) for different usecases? I do not really like to use the inspect module there. If you have a good reason, it might be better to just change the API of the BaseMarkup class, to include the request (and optionally args/kwargs) for all conversion methods.

    For the changes we should have some tests. As soon I have your answer, I will update them a bit.

    Apart from that, I think markupmirror should be updated to support Python 3, Django 1.5 and the newer versions of textile, markdown, docutils and FeinCMS - also it should use the new version of Codemirror.

    I am currently not using markupmirror myself in any of my projects, so my motivation is a bit low. Are you interested in putting some time into it?

    Regards, Fabian

  3. Stephen Moore author

    Yeah, I was a bit lax with making tests, sorry about that :p

    I need the request because I need to know what instance of the model is being edited when I do the conversion (I have special markup things that change depending on values on some related models.) I also have a local mod to the javascript that edits the post data (a bit too hacky for a pull request atm). the use of inspect was tomake the change compatible with existing converters.

    also, unfortunately, I'm only using this for a hobby like project of mine, so my available time for doing much more to this is tiny...

    anyway, it's 2am in my timezone atm, goodnight!

  4. Fabian Büchler repo owner

    Hi Stephen,

    just for the record, I am still working on this. Have a bit of a tight schedule currently, but I will finish this and publish it. Also I'm planning to do an update for Django 1.5 subsequently.

    Regards, Fabian

Tip: Filter by directory path e.g. /media app.js to search for public/media/app.js.
Tip: Use camelCasing e.g. ProjME to search for ProjectModifiedEvent.java.
Tip: Filter by extension type e.g. /repo .js to search for all .js files in the /repo directory.
Tip: Separate your search with spaces e.g. /ssh pom.xml to search for src/ssh/pom.xml.
Tip: Use ↑ and ↓ arrow keys to navigate and return to view the file.
Tip: You can also navigate files with Ctrl+j (next) and Ctrl+k (previous) and view the file with Ctrl+o.
Tip: You can also navigate files with Alt+j (next) and Alt+k (previous) and view the file with Alt+o.