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

clean up how the percent directives at the top of cells are processed #5050

Closed
mwhansen opened this issue Jan 22, 2009 · 7 comments
Closed

Comments

@mwhansen
Copy link
Contributor

Component: notebook

Issue created by migration from https://trac.sagemath.org/ticket/5050

@mwhansen
Copy link
Contributor Author

Attachment: trac_5050.patch.gz

@jasongrout
Copy link
Member

comment:1

Notes from Mike looking at this:

  • Docs for parse_percent_directives needs to give what is returned
  • is_html doesn't need to call parse_percent_directives
  • url_to_self should have at least one line of documentation besides the examples
  • %hide and %hideall can also be cleaned up from the html and html_in methods (maybe other places)

@mwhansen
Copy link
Contributor Author

Attachment: trac_5050-2.patch.gz

@jasongrout
Copy link
Member

comment:2

One positive review. It's probably important enough that at least one other person ought to review it as well.

Mike and I stepped through the changes, they all look good, and I tested for about 20 minutes with various combinations of the directives and things seemed to work. I had one glitch, but I can't reproduce it. I had a cell that looked like:

%hide
%hideall
print "hi"

I closed the cell, reopened it, then deleted all output, then did a few more things I can't remember. At one point, the cell changed to:

%hide
%hid

If we can't reproduce something like this, though, I give it a positive review.

@jasongrout
Copy link
Member

comment:3

In the above glitch, I mean to say that I closed the worksheet, reopened it, etc...

@rlmill
Copy link
Mannequin

rlmill mannequin commented Jan 22, 2009

comment:4

Applies cleanly to Sage 3.3.alpha0, passes doctests, and seems to be working when I test it out. This is on Intel OSX 10.5.

@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Jan 23, 2009

comment:5

Merged in Sage 3.3.alpha1

@sagetrac-mabshoff sagetrac-mabshoff mannequin closed this as completed Jan 23, 2009
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants