interact : incomplete default string in the input box

[seblabbe]

Using interact in sage 3.1.4, the default string doesn't print completly in the input box. It looks like it prints up to the first character ' found.
The folowing example works well

@interact
def _(a=input_box(default='interact is "cool"',type=str,label='Name:')):
    print a

and it puts interact is "cool" in the input box. But in the next one,

@interact
def _(a=input_box(default="interact is 'cool'",type=str,label='Name:')):
    print a

the default string in the input box is incomplete, it puts only *interact is *. So, we don't know if interact is cool or not !

Component: interact

Keywords: input_box

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

[mwhansen]

Attachment: trac_4524.patch.gz

[seblabbe]

comment:2

First, that is the first time I review a patch. So if you have comment on the way I review, tell me. I also have some questions that I write below as I have them. Moreover, I am not familiar with notebook code at all....so should I let the job to somebody else?

However, I applied the patch and I can at least say that the problem I submitted is now fixed which is cool!

There is a small issue here (double that) :

Note that any HTML that that uses quotes around this should use double quotes and not single quotes. 

Also, I don't know if you agreee, but I suggest to add the second example below :

EXAMPLES: 
    sage: from sage.server.notebook.interact import InteractControl 
    sage: InteractControl('x', '"cool"').html_escaped_default_value() 
    '"cool"' 
    sage: InteractControl('x',"'cool'").html_escaped_default_value()
    "'cool'"

My last question is : I could have posted a patch for both issues (double that, and the 2nd example). What is commonly done? Do we leave the changes to the patcher?

My statement is Positive review pending fixes on at least the first of my two doc-suggestions.

Thanks for the fix,

[mwhansen]

comment:3

I've posted a patch which addresses the issues.

You could have posted a patch and then asked me to review your portion of the patch.

[seblabbe]

comment:4

Attachment: trac_4524-2.patch.gz

Positive review.

[seblabbe]

comment:5

I think it can be merged in sage 3.3.

[sagetrac-mabshoff]

comment:6

Merged in Sage 3.3.alpha2