acoomans / flask-autodoc

Flask autodoc automatically creates an online documentation for your flask application.
MIT License
98 stars 49 forks source link

Custom Function Properties #14

Closed BallisticBuddha closed 9 years ago

BallisticBuddha commented 9 years ago

Added variable args to the doc decorator that add custom variables to a route's documentation.

These variables are passed into the documentation that is generated and can be used in the template to have more customized and structured data (e.g. query string parameters, post form data, expected content types, etc.) without having to put everything in the docstring, making those properties easier to deal with when working with a template.

The default template has also been modified to display these custom properties in a similar fashion as the default ones. However, the data is currently only displayed as raw string conversions from python, with no formatting changes.

Additionally, some existing function properties can be overridden by specifying them in the decorator. I hardcoded a variable in autodoc that defines which properties cannot be overridden, these are currently 'rule' and 'endpoint'.

lukeyeager commented 9 years ago

Hi @BallisticBuddha,

This looks like a sold improvement. If you can address my small suggestion, rebase to master, and squash into a single commit, I'd be happy to merge this.

BallisticBuddha commented 9 years ago

I have finished rebasing my changes against master. Integration seems to have gone well.

BallisticBuddha commented 9 years ago

Okay, so I have complied with your request to remove all of the custom property stuff from the default template but this ended up exposing two new issues with this change:

1) For some reason, I cannot for the life of me manage to squash out the resolution of a merge conflict that occurred now that PR #21 has been merged in. No matter how many ways I tried to rebase and squash out that merge conflict, I simply cannot get rid of that merge commit from the history while still being able to push the newly rebased commit into my fork.

2) I ended up having to remove that last unittest I made that tests HTML with custom params. There are a slew of complications when you try to use a jinja2 template that exists outside of the server's root path. And according to the official flask documentation here: http://flask.pocoo.org/docs/0.10/quickstart/#rendering-templates It seems like pretty much the only way to do this is to make a different app that exists somewhere else. However, I also came to realize that Flask does not support relative imports when creating an app, so making a app_custom during the setup of the unittest is not going to happen. I really would like that unittest to persist, but my goodness this got ridiculously complicated.

So, with that unittest gone, I went ahead and added some example of using custom params to the custom blog.py example, offering some way to test this stuff.

lukeyeager commented 9 years ago

1) Try this:

# grab the latest changes
git remote update
# move to your feature branch
git checkout customProperties
# place your changes after origin/master
git rebase origin/master
# squash your changes into a single commit
git rebase -i origin/master

2) Wow, that sounds like a mess. Don't try to do anything too fancy - this should be a pretty simple little package. It looks like you've got it working - what is missing?

BallisticBuddha commented 9 years ago

1) Rebasing itself wasn't the problem, I could rebase and squash out anything I wanted all day, but the problem arose when I tried to push the amended commit up to my fork. Since you seem to be very insistent on making all of this exactly cone commit, even if the extra commit was just a merge conflict that needed to be there in order to merge back into master correctly, I was forced to re-author my commit, therefore losing all of the history of commits I made back in January. The squashed commit must either A) be succeeded by a merge commit that informs git how to resolve the conflict (regular non-merge commits cannot do this) or B) be authored after a49f384b470e8fb7cd5b059ae19d9b26bf90b6b0 was authored, therefore avoiding the conflict.

So again, to comply with this request, I had to re-author the commit, which now has a commit date of today, rather than in January when I actually wrote all this code. I was trying to avoid this, as I hate falsifying commit dates, but if you really need the commit history to be as pristine as possible, I am willing to get over this pet peeve of mine.

2) Yes, it is quite a mess, and in the name of keeping something that should be simple simple, I had to omit the test case I made called "testHTMLWithCustomParams". I do still have my test cases in the test suite that ensures that custom prams and override params can be put into the doc decorator, called testCustomParams and testOverrideParams. What I removed was the test that makes sure that these custom params actually get put into the HTML document that is generated.

This makes this feature have about equal coverage as the "location" feature you merged in not too long ago. I don't know if you ever tried to make a test case that tests to make sure that the location line is actually put into the generated HTML document, but if you ever do, you will likely run into this madness as well.

lukeyeager commented 9 years ago

1) Rebasing

I had to re-author the commit, which now has a commit date of today, rather than in January when I actually wrote all this code. I was trying to avoid this, as I hate falsifying commit dates, but if you really need the commit history to be as pristine as possible, I am willing to get over this pet peeve of mine.

Ah ok, I didn't realize you cared about that. You're the first I've encountered with that particular pet peeve. Thanks for putting up with mine about commit history :stuck_out_tongue_winking_eye:

2) Testing

I don't know if you ever tried to make a test case that tests to make sure that the location line is actually put into the generated HTML document, but if you ever do, you will likely run into this madness as well.

Ah, no I didn't test it in the HTML. This is as far as I got w.r.t. testing. (NOTE: I'm waiting to merge this before merging that in #19)