Ticket #11000 (closed defect: fixed)

Opened 2 years ago

Last modified 2 years ago

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: Work issues:
Report Upstream: N/A Reviewers: Rob Beezer, Keshav Kini
Authors: Ivan Andrus Merged in: sage-4.7.alpha4
Dependencies: Stopgaps:

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

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

Change History

Changed 2 years ago by iandrus

comment:1 Changed 2 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 2 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 2 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 2 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 2 years ago by kini

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

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 2 years ago by jdemeyer

  • Authors changed from iandrus to Ivan Andrus

comment:7 Changed 2 years ago by jdemeyer

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