Opened 10 years ago

Closed 10 years ago

#11000 closed defect (fixed)

GAP interface doesn't handle input with multiple lines correctly

Reported by: iandrus Owned by: iandrus
Priority: major Milestone: sage-4.7
Component: interfaces Keywords: gap
Cc: Merged in: sage-4.7.alpha4
Authors: Ivan Andrus Reviewers: Rob Beezer, Keshav Kini
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

The following GAP input (say in the notebook)

if 4>3 then
Print("You should see me.");
fi;

gets changed to

if 4>3 thenPrint("You should see me.");fi;

before sending to GAP. Obviously this fails since thenPrint should be two words.

It also doesn't strip comments correctly as can be seen by

## here is a comment
if 4>3 then
 Print("here");
fi

which returns nothing.

Attachments (1)

trac_11000.patch (4.3 KB) - added by iandrus 10 years ago.

Download all attachments as: .zip

Change History (8)

Changed 10 years ago by iandrus

comment:1 Changed 10 years ago by iandrus

  • Status changed from new to needs_review

The reason they didn't send it all at once seems to be that the interface doesn't properly handle continuation prompts (it deletes enough characters for the whole prompt). Sending all on one line avoids them... except of course when it doesn't. In particular something like

if true then

in the notebook will cause all sorts of unhappiness (3 characters cut off the end of the output) until a cell like

fi

is evaluated. Hopefully this should be fixed now. We probably shouldn't worry too much about the above case since a similar cell with

if false then

will simply cause everything to stop being evaluated until a closing cell is evaluated. I don't see a way around this, and it should probably be considered user error.

comment:2 Changed 10 years ago by kini

I'm getting doctest errors in, I think, four files, but I stupidly didn't log them. Confirming...

(does this patch depend on any other patches? I'm running it on 4.7.alpha3)

comment:3 Changed 10 years ago by kini

... OK, I ran the test again.

The following tests failed:

        sage -t -long 4.7.alpha3-main/devel/sage-main/sage/interfaces/maxima.py # 4 doctests failed
        sage -t -long 4.7.alpha3-main/devel/sage-main/sage/tests/startup.py # 1 doctests failed

The startup.py failure is not important, and the maxima failure seems to remain even when I unapply your patch and rebuild, so I have no idea what's going on with that.

comment:4 Changed 10 years ago by rbeezer

This passes all tests for me, plus the two modules Kini mentioned above. Parallel testing on 4.7.alpha1, 64-bit Ubuntu.

comment:5 Changed 10 years ago by kini

  • Reviewers set to Rob Beezer, Keshav Kini
  • Status changed from needs_review to positive_review

OK, looks all clear to me. Works as advertised, tested manually in the console too. Positive review. I guess the maxima thing is just something weird about alpha2 or alpha3, maybe specific to sage.math.washington.edu, since you guys are using alpha1.

comment:6 Changed 10 years ago by jdemeyer

  • Authors changed from iandrus to Ivan Andrus

comment:7 Changed 10 years ago by jdemeyer

  • Merged in set to sage-4.7.alpha4
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.