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
magma mode in 4.7 notebook broken #11401
Comments
comment:1
Could this be caused by #9705? |
comment:2
I guess it could be caused by its fix ... The cells there execute without problem for me in 4.7 and those cells do contain multiple commands that do not produce output. It seems that the input (in one cell)
reliably knocks the interface out of sync. So probably if someone who know how these interfaces work, looks at the strings that are passed back and forth, it will become apparent quite quickly. I don't think the problem is confined to the notebook either. I observed:
|
straightforward fix |
comment:3
Attachment: trac_11401.patch.gz Perhaps a little too naive of a fix and perhaps focussing a little too much on the symptom rather than the cause, but if newlines are causing problems, why not just replace them with spaces? In magma syntax, both newline and space are whitespace characters and those are all treated the same.
which I suppose contains all magma interfacing doctests? |
comment:4
I think I now have a proper fix for the problem. The problem is indeed caused by the fix of #9705.
The solution in This means that the I have also added a doctest testing that #11401 is fixed. I also reintroduced the newlines in the test for #9705. Those are actually essential for testing (the version without newlines always worked as expected). |
This comment has been minimized.
This comment has been minimized.
comment:7
I don't have a machine without Magma to test this on, but I'm pretty sure that the examples " ps. for the patchbot which didn't seem to have gotten the message: |
Author: Nils Bruin |
Reviewer: Marco Streng |
Attachment: trac_11401v2.patch.gz (commit message + "optional -- magma" for examples) |
comment:8
Good catch! fixed. (amnesic patchbot: apply trac_11401v2.patch) |
comment:10
Why are you changing the example for "Verify that trac 9705 is fixed"? I don't see why one is better than the other. To check for myself that your new version of that test really verifies that trac 9705 is fixed, I would have to install a pre-#9705 version of Sage and see that that test fails. That's a bit more work than I would like to do. Patchbot, would you apply trac_11401v2.patch please? |
comment:11
All tests pass. Patch looks good. I haven't tried the notebook, but the patch does what it should do to multi-line magma.eval. That leaves just the one question about the changed #9705 doctest. |
comment:12
Replying to @mstreng:
Because the example without newlines runs OK both with and without the fix of #9705, so the test is not verifying any change in behaviour. The problem described in #9705 arose when one magma statement was spread over multiple lines and one line was passed over stdin and another line via a "load" command. Without newlines, such splits wouldn't occur anyway. If people can't find a fix for the "\n" in the tests, I guess we could write it in the following way:
|
Attachment: trac_11401v2.2.patch.gz compliant commit-message and fix for newlines |
comment:13
OK, latest version should provide proper formatting of documentation Dear patchbot, would you be so kind to just apply trac_11401v2.2.patch please? |
comment:14
That new version looks good, and does the trick. I'm glad you didn't really do |
This comment has been minimized.
This comment has been minimized.
comment:15
Please use proper URLs, or in this case, trac wiki mark-up (" |
Merged: sage-4.7.2.alpha3 |
See also
this sage-devel thread
It seems that magma mode in the 4.7 notebook is broken. I observed the following session after verifying John Cremona's report about syncing problems. I suspect sync is lost due to the multiple commands. Note that none of the print commands seem to get executed (no output is observed) and that for "print 7;" the line "print 6;" gets echoed (this has probably very little to do with the "print 7;" command itself)
Apply only attachment: trac_11401v2.2.patch to the Sage library.
Component: interfaces
Keywords: magma notebook interface
Author: Nils Bruin
Reviewer: Marco Streng
Merged: sage-4.7.2.alpha3
Issue created by migration from https://trac.sagemath.org/ticket/11401
The text was updated successfully, but these errors were encountered: