Fix PrintFormattingStatus and SetPrintFormattingStatus for the standard output (i.e., when the first argument is "*stdout*" and add a variant that deals with the current output (whatever that may be), reachable by using "*current*" as first argument

[fingolfin]

Extracted from PR #4540 -- these are the hopefully unproblematic bits. Fixes issue #4486 (UPDATE: no it doesn't) (CC @zickgraf)

Description

Text for release notes

Fix PrintFormattingStatus and SetPrintFormattingStatus for the standard output (i.e., when the first argument is "*stdout*" and add a variant that deals with the current output (whatever that may be), reachable by using "*current*" as first argument

(End of text for release notes)

[zickgraf]

Thanks a lot for pushing this! I just tried it and now SetPrintFormattingStatus indeed works for stdout as expected (multiple stdouts are not synced, but that's not supposed to be in this PR if I understand correctly). Formally, this PR does not fix my issue #4486 but that's due to my issue not being well-defined, so we can as well close it :D

[fingolfin]

(multiple stdouts are not synced, but that's not supposed to be in this PR if I understand correctly).

I am not sure what makes you think that, could you elaborate? Ohh, I see: any new stdout will still use a fixed default value. So we also need to store the value somewhere (maybe reintroduce GAPInfo.FormattingStatusStdout after all; though I think a kernel variable would be better, to enforce a consistent state).

BTW, so far all other SetPrintFormattingStatus methods do not affect streams that are already open; with this PR, both SetPrintFormattingStatus("*stdout*", val) and SetPrintFormattingStatus("*current*", val) do that. Perhaps the other methods should, too?

Formally, this PR does not fix my issue #4486 but that's due to my issue not being well-defined, so we can as well close it :D

Ah right, my bad, I was confused about the content of your issue. I'd like to leave it open for now.

[zickgraf]

Ohh, I see: any new stdout will still use a fixed default value.

Exactly! A specific example/use case is: I want to disable the print formatting for all stdouts by putting SetPrintFormattingStatus( "*stdout*", false ); in my gaprc. But this does not work, I assume because the REPL opens another stdout with default formatting status.

BTW, so far all other SetPrintFormattingStatus methods do not affect streams that are already open

I don't understand this statement. Consider the following code:

gap> string := "";
""
gap> stream := OutputTextString( string, false );
OutputTextString(0)
gap> PrintTo( stream, x -> x );
gap> SetPrintFormattingStatus( stream, false );
gap> PrintTo( stream, x -> x );
gap> CloseStream( stream );
gap> Display( string );
function ( x )
    return x;
endfunction ( x )
return x;
end

SetPrintFormattingStatus affects stream, which I consider being "open" at that point. Or is it possible to have multiple copies of stream like for stdout and your statement is that other copies are not affected?

Ah right, my bad, I was confused about the content of your issue.

The problem with my issue is that the "observed behavior" is actually correct :D Should I rename it to "Feature Request: Allow to disable line wrapping but keep indentation when printing functions"?

[fingolfin]

SetPrintFormattingStatus affects stream, which I consider being "open" at that point

The meaning of "open" I had in mind is: active on GAP's stack of output streams. Which it is not: only Print will put it there, temporarily, then print, then remove it again. If any code executed in the meantime which modifies the print formatting status, it won't have any effect. That said, in "natural" examples this should never matter, so it's probably OK.

I'll fix this PR to make the formatting status for stdout persistent.

Now I wonder if it should also affect errout; or if perhaps errout should have a separate persistent state, and SetPrintFormattingStatus("*errout*", val) should become a thing?

[zickgraf]

The meaning of "open" I had in mind is: active on GAP's stack of output streams. [...] That said, in "natural" examples this should never matter, so it's probably OK.

Ah, thanks for the explanation! Yes, I see no application where this would matter (for me).

Now I wonder if it should also affect errout; or if perhaps errout should have a separate persistent state, and SetPrintFormattingStatus("*errout*", val) should become a thing?

I don't use errout, so I don't really have an opinion on this. Assuming that errout is only used rarely: maybe it would make sense to ignore errout completely for now (i.e. SetPrintFormattingStatus("*stdout*", val) should NOT affect errout), and wait if the need for a separate setting arises in the future?

[fingolfin]

Break loops use errout

[zickgraf]

Ah, sorry, I should have guessed that. One reason why I want to disable the line wrapping is to make GAP not break and insert backspaces in filenames in stack traces, which is of course highly relevant in break loops. So with that new piece of information I think having the possibility to set the line wrapping of errout would be nice indeed. It does not matter for my application whether this would be included in *stdout* or available as a separate setting, since I want to disable it for both anyway.

[fingolfin]

Since we were discussing also handling *errout*, perhaps there should be a single pair of kernel function (SET_)PRINT_FORMATTING_STATUS(name, ...) which handles *stdout*/*errout*/*current* plus future additions?

Ah well, I guess it's not really important whether we do it one way or another shrug

[ChrisJefferson]

This is good, there are some small typos I've referenced, but nothing serious.

[frankluebeck]

Here are a number of comments:

1. The purpose of the function PrintFormattedString is to print strings without unwanted line breaks which are longer than the current screen width (e.g., they contain UTF-8 encoded characters or ANSI escape sequences). The function was implemented by printing additional flush characters \c which as a side effect also reset the column counter (side remark: if this behaviour of \c was changed it would certainly break some of my code; if we want a "flush only" character we could introduce a new one, say \f.) When PrintFormattedString was written GAP had no possibility to change the formatting status of the standard output.
2. In the current GAP master SetPrintFormattingStatus( "*stdout*", false ); is broken - and this is fixed by this pull request (now it looks again as it was in GAP 4.6.7).
3. I suggest to introduce a new function in the GAP library (adjust name as you like, maybe also a PrintToStringNoFormat variant):

PrintStringNoFormat := function ( str )
    local stat;
    stat := PrintFormattingStatus( "*current*" );
    SetPrintFormattingStatus( "*current*", false );
    Print( str );
    SetPrintFormattingStatus( "*current*", stat );
end;

4. Then the function in 3. could be used where we used PrintFormattedString before (instead of using the code of the function in several places). And I could bind PrintFormattedString to it in the GAPDoc package.
5. The output of Print(x->x);, depending on the formatting status, looks wrong to me; but this should be discussed elsewhere. Therefore, I would choose a better example in the test files to demonstrate/test SetPrintFormattingStatus, e.g.:

gap> sc := SizeScreen();; SizeScreen([20,sc[2]]);;
gap> [0..16]+0;
[ 0, 1, 2, 3, 4, 
  5, 6, 7, 8, 9, 
  10, 11, 12, 13, 
  14, 15, 16 ]
gap> old := PrintFormattingStatus("*stdout*");
true
gap> SetPrintFormattingStatus("*stdout*", false);
gap> [0..16]+0;
[ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 ]
gap> SetPrintFormattingStatus("*stdout*", true);
gap> SizeScreen(sc);;

6. I would vote for handling *stdout* and *stderr* in the same way, but as separate output streams.
7. When the separation in 6. is available: Shouldn't the DefaultInfoHandler print to *stderr*?
8. In DefaultInfoHandler: Shouldn't the formatting be switched off for any output stream (not just *Print*)?
9. I do not understand the precise purpose of the *current* pseudo output stream (and so how useful it may be to introduce it).

[fingolfin]

@frankluebeck some replies

re 1: understood; note that this PR does not change how we treat \c, PR #4540 does (which I put on hold for now, so we can first focus on getting those bits ready and merged which are uncontroversial and don't break existing tests).

re 3: how about PrintUnformatted?

re 6: then we agree

re 7: maybe? I am fine either way (I rarely use GAP with stdout different from errout)

re 8: again: maybe? we are setting the rules here. I wondered about it, too, but decided that I should keep the changes to a minimum; in that spirit, I'd prefer if such a change to DefaultInfoHandler would be done in a separate PR

re 9: *current* allows modifying the formatting status of whichever stream is active, which we normally cannot do. One application is of course the functionPrintStringNoFormat/ you proposed PrintUnformatted. I guess we could also implement that one purely in the kernel, without needing to introduce *current*... Another application I had in mind was to allow using it in .tst files, so that one can temporarily disable line wrapping for a test. This kinda somewhat works, but only if the SetPrintFormattingStatus("*current*",false); is in the same line as the calls it is supposed to affect (e.g. Display(x -> x);, see tst/testinstall/print-formatting.tst). This is because for tests, we create a fresh output stream for each test input (not sure why)

[fingolfin]

I've added PrintUnformatted here now - it is intentionally undocumented for now, but we can of course change that later.

I'll add support for setting the print formatting status for *errout* in a future PR (it'll be easy enough but I'd prefer not to complicate and delay this PR further, plus it'll make it easier to keep track of the changes for the release notes.

As such, I'd like to merge this as soon as CI passes; we can make further tweaks later.

[frankluebeck]

@fingolfin :

re re 1: Yes my "side remark" was concerning PR #4540.

re re 3: PrintUnformatted can be misunderstood as its argument being "not formatted" (the 'format' here has a different meaning than the Format part in PrintFormattedString, which refers to pre-formatted text). How about PrintWithoutFormatting which would fit with (Set)PrintFormattingStatus?
Actually, this function makes sense for printing anything, not just pre-formatted strings (you have used arg instead of str).
It really should be documented such that I can use it in GAPDoc.

re re 7 and 8: Agreed, this can be done in a different PR.

[fingolfin]

re re re 3: agree, renamed to PrintWithoutFormatting. I've not yet documented it -- this, too, can come in a later PR (you can already use it in GAPDoc, though)