Ticket #5050 (closed defect: fixed)

Opened 4 years ago

Last modified 4 years ago

[with patch, positive review] clean up how the percent directives at the top of cells are processed

Reported by: mhansen Owned by: boothby
Priority: major Milestone: sage-3.3
Component: notebook Keywords:
Cc: Work issues:
Report Upstream: Reviewers:
Authors: Merged in:
Dependencies: Stopgaps:

Description


Attachments

trac_5050.patch Download (15.0 KB) - added by mhansen 4 years ago.
trac_5050-2.patch Download (2.6 KB) - added by mhansen 4 years ago.

Change History

Changed 4 years ago by mhansen

comment:1 Changed 4 years ago by jason

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)

Changed 4 years ago by mhansen

comment:2 Changed 4 years ago by jason

  • Summary changed from clean up how the percent directives at the top of cells are processed to [with patch, needs review] clean up how the percent directives at the top of cells are processed

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.

comment:3 Changed 4 years ago by jason

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

comment:4 Changed 4 years ago by rlm

  • Summary changed from [with patch, needs review] clean up how the percent directives at the top of cells are processed to [with patch, positive review] clean up how the percent directives at the top of cells are processed

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.

comment:5 Changed 4 years ago by mabshoff

  • Status changed from new to closed
  • Resolution set to fixed

Merged in Sage 3.3.alpha1

Note: See TracTickets for help on using tickets.