AspenWeb / pando.py

Filesystem dispatch + Simplates + Python = a nice web framework.
http://aspen.io/
Other
149 stars 38 forks source link

use a different separator than page break (^L) #167

Closed chadwhitacre closed 11 years ago

chadwhitacre commented 11 years ago

Pretty much since the beginning people have been saying that ASCII page breaks are too weird. We relaxed the constraint to caret-ell as a result. @dcrosta rewrote the framework more or less to get a different separator. Whatever it is in Ruby that does something similar uses something different. The latest thready discussion is leaning the same way:

http://www.reddit.com/r/Python/comments/1b3t09/why_gittip_use_aspen_instead_of_djangorails/

So let's maybe do that. How about the separator is \n----+(.*)\n, where the group is used as the specline (specifying which syntax the following page is).

chadwhitacre commented 11 years ago

Thanks for chiming in, @warsaw. Love the history! :D

@Lucretiel Re: question in IRC ... padding is so we get accurate line numbers in tracebacks. We haven't had tests for that but we should.

chadwhitacre commented 11 years ago

@pjz You up to review and merge @Lucretiel's work?

chadwhitacre commented 11 years ago

@Lucretiel I gave your branch a once-over and it looks good on the surface. Is this ready for a full review? What are you seeing as next steps?

Lucretiel commented 11 years ago

Yeah I'd say it's good to go at this point. The only thing still missing finishing updating some of the negotiated_template tests

pjz commented 11 years ago

Looks good, so I merged it; re-open this if there are issues with it.

chadwhitacre commented 11 years ago

Damn. Good honkin' work, everyone! Big thanks to @Lucretiel for implementing this and to @pjz for reviewing/merging.

The tests are passing for me and I'm seeing [-----------------] in the simplates in docs/. Did some poking at speclines and that seems to be working as well.

We need to update the docs, themselves, though. We're still talking about ^L. There's a make doc target to run the docs locally from a clone.

chadwhitacre commented 11 years ago

Escaping doesn't work.

Lucretiel commented 11 years ago

Can you give an example? The tests were (I believe) passing. On Apr 15, 2013 2:42 PM, "Chad Whitacre" notifications@github.com wrote:

Escaping doesn't work.

— Reply to this email directly or view it on GitHubhttps://github.com/gittip/aspen-python/issues/167#issuecomment-16403633 .

chadwhitacre commented 11 years ago

@Lucretiel Am I reading that wrong?

chadwhitacre commented 11 years ago

I noticed this while updating the docs.

chadwhitacre commented 11 years ago

@Lucretiel Is this something you think you'll be able to take a look at? I leave for NYC early tomorrow. Would love to have this released and implemented on Gittip before tomorrow night's talk at NYCPython.

Lucretiel commented 11 years ago

Keep in mind that the check_page_content helper only tests paginating, not escaping. I don't believe I have any tests written that test splitting and escaping together; recall that they are separate operations, and only sewn together by the simple method split_and_escape. I'm working on a test that ensures that the escaped lines are left untouched.

Lucretiel commented 11 years ago

Bug found. My code, and all my tests, use / as the escape character, not . Fixing.

pjz commented 11 years ago

So the issue here is that @Lucretiel wrote escaping with forward slashes and @whit537 tried to escape using backslashes. The docs in this issue say to implement using backslashes. My bad for not catching that in code review. @Lucretiel think you can fix it and shoot me a pull request?

chadwhitacre commented 11 years ago

Phew. :)

chadwhitacre commented 11 years ago

@Lucretiel @pjz I'll stop with the changes to master for now while we fix this.

chadwhitacre commented 11 years ago

I'm going to start looking at #148 but I'll lay off master.

chadwhitacre commented 11 years ago

@Lucretiel Also, may I add you as a collaborator on the Gittip org with perms on this repo (aspen-python)? You do good work. :)

Lucretiel commented 11 years ago

By all means

Sent from my Galaxy Nexus On Apr 15, 2013 3:25 PM, "Chad Whitacre" notifications@github.com wrote:

@Lucretiel https://github.com/Lucretiel Also, may I add you as a collaborator on the Gittip org with perms on this repo (aspen-python)? You do good work. :)

— Reply to this email directly or view it on GitHubhttps://github.com/gittip/aspen-python/issues/167#issuecomment-16406109 .

chadwhitacre commented 11 years ago

@Lucretiel Done. Welcome aboard. :)

P.S. You should consider joining Gittip when you get a chance:

https://www.gittip.com/on/github/Lucretiel/

pjz commented 11 years ago

bugs fixed as of ee371e4eda. Still need docco update.

pjz commented 11 years ago

Actually looks like @whit537 fixed the docs yesterday. Closing this unless/until other bugs found.

Lucretiel commented 11 years ago

Changed to be a raw string. Regarding @whit537's regression commit: I originally had that stuff in a different module, but I didn't want to disrupt the model of all the modules in resources being resource types.

Lucretiel commented 11 years ago

Reverted

pjz commented 11 years ago

Ugh, I owe you an apology. Turns out that while r'' keeps the python interpreter from turning \n into a newline, the regex interpreter will then do it. So it was working after all. either way works; escape hell is still escape hell :)

chadwhitacre commented 11 years ago

@pjz sez we're not allowed to change this again for at least 6 months. :)