#4132 breaks indentation in displayed functions in certain cases

[zickgraf]

Put the following code in a file test.g:

SetPrintFormattingStatus( "*stdout*", false );
Display(x -> x);

and execute gap test.g.

Observed behaviour

The output is:

function ( x )
return x;
end

Expected behaviour

The output should be:

function ( x )
    return x;
end

Notes

1. I have bisected this to #4132 (e62d39555838725badc50c716a29cdbf7cc576c4).
2. In the REPL this problem does not occur, that's why putting the code into a file is important.
3. Some background why I am interested in this:
   1. I have SetPrintFormattingStatus( "*stdout*", false ); in my gaprc because I'd like my terminal to reflow long lines if I change the size of my terminal.
   2. I want to stringify functions and print them to files. For this, I would like to have indentation but not line breaks due to long lines, so I would like to set SetPrintFormattingStatus to false without losing the indentation.

[ChrisJefferson]

I didn't realise SetPrintFormattingStatus on stdout ever did thisl.

The formatting for line breaking and indentation are generally mixed together -- for example if I go back to GAP 4.10 (from 2019), or use current the current master, and use PrintTo to print functions to files, when I set PrintFormattingStatus to false, it disables both line wrapping and function indentation.

So, I think the behaviour you observed, while I agree a useful behaviour, might have technically been "a bug" (certainly SetPrintFormattingStatus should either effect all function indenting, or none, regardless of how functions are printed).

I do think generally "fixing" this would be good, and separating these two concepts. However, the problem is to decide what various options should do -- should we always indent functions regardless of formatting status for example? Or have another option to change the behaviour of that?

[zickgraf]

Mhh, I see. I cannot think of a situation where I not would want indentation, so "always indenting functions regardless of formatting status" sounds sensible to me. (At least, removing the indentation if one does not need it is easy, while adding it is really hard :D )

But seeing that this might be more complicated than I expected: Could one simply increase the limit imposed on the number of columns in SizeScreen? In my applications I deal with lists of string with >200 entries. Thus, the limit 4096 is a bit small (~ 20 characters per string), but 16384 or 32768 should suffice. (Of course I understand if you do not want to raise this limit arbitrarily to fit a random application.)

[fingolfin]

Huh, I am confused. @zickgraf says he bisected this to PR #4132. But as a matter of fact, with the master branch, I cannot reproduce the issue:

gap> SetPrintFormattingStatus( "*stdout*", false );
gap> Display(x -> x);
function ( x )
    return x;
end

However, I can reproduce it in stable-4.11 -- which does not contain PR #4132.

[zickgraf]

From reading your code section I assume that you have tried this in a REPL? My issue only occurs if the code is in a file. But your test is still interesting, it shows that before #4132 the problem was reversed (i.e., there was indentation when reading from a file, but not in a REPL). Here is an output of all combinations:

$ cat test.g
SetPrintFormattingStatus( "*stdout*", false );
Display(x -> x);

$ gap test.g
 ┌───────┐   GAP 4.12dev-566-g56dce07 built on 2021-05-17 16:42:53+0200
 │  GAP  │   https://www.gap-system.org
 └───────┘   Architecture: x86_64-pc-linux-gnu-default64-kv8
 Configuration:  gmp 6.2.1, GASMAN, readline
 Loading the library and packages ...
 Packages:   Browse 1.8.9, GAPDoc 1.dev, IO 4.7.0dev, PrimGrp 3.4.0, SmallGrp 1.4.1, TransGrp 2.0.5
 Try '??help' for help. See also '?copyright', '?cite' and '?authors'
function ( x )
    return x;
end
gap> Display(x -> x);
function ( x )
    return x;
end
gap> SetPrintFormattingStatus( "*stdout*", false );
gap> Display(x -> x);
function ( x )
return x;
end

$ gap test.g
 ┌───────┐   GAP 4.12dev-567-ge62d395 built on 2021-05-17 16:43:45+0200
 │  GAP  │   https://www.gap-system.org
 └───────┘   Architecture: x86_64-pc-linux-gnu-default64-kv8
 Configuration:  gmp 6.2.1, GASMAN, readline
 Loading the library and packages ...
 Packages:   Browse 1.8.9, GAPDoc 1.dev, IO 4.7.0dev, PrimGrp 3.4.0, SmallGrp 1.4.1, TransGrp 2.0.5
 Try '??help' for help. See also '?copyright', '?cite' and '?authors'
function ( x )
return x;
end
gap> Display(x -> x);
function ( x )
    return x;
end
gap> SetPrintFormattingStatus( "*stdout*", false );
gap> Display(x -> x);
function ( x )
    return x;
end

Commit e62d395 is the one from #4132.

[fingolfin]

Ah indeed I was trying the REPL, oops. But this issue only happend for me with 4.11, it was fine in 4.10. Haven't tried file mode or older versions yet

[ChrisJefferson]

Just a comment, I believed PrintFormattingStatus always disabled indentation (I may be wrong).

The reason the indentation appears in the REPL is because SetPrintFormattingStatus is being ignored. You can see this by printing something like Display(x -> xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx);, and observing it still has line wrapping, putting \ at the end of the line.

[ChrisJefferson]

I investigated this in #4496 . That PR probably isn't final. In particular, it would be a change to make SetPrintFormattingStatus leave function indentation (although, I think it would be an entirely sensible change).

[frankluebeck]

GAP uses the characters \< and \> als line break hints. If a line becomes too long, GAPs output algorithm uses these characters to decide a good point to break a line and in this case the size of the indentation of the next line. (The precise rules are complicated and I forgot the details.)

The GAP kernel code that prints a function body was written at a time when nobody thought about SetPrintFormattingStatus which with argument false causes to just ignore the line break hints.

A part of what happens can be seen in the function PrintFunction in src/calls.c: There is a mixture of explicit line breaks with \n and \<, \> characters which in this case are rarely used as line break hints (because there are frequent \ns) but (maybe nowadays mis-)used to determine the indentation of a new line.

When the formatting status is false then this implementation leads to unpleasant and (I think) unintended output of function bodies.

I'm not sure how this should be changed. There is probably no general agreement which of

gap> Print(x->x);
function ( x )
    return x;
end
gap> fu := function(n) local a; a := n^2; if a < 100 then return a; else return 2*a; fi; end;
function( n ) ... end
gap> Print(fu);
function ( n )
    local a;
    a := n ^ 2;
    if a < 100 then
        return a;
    else
        return 2 * a;
    fi;
    return;
end

or

gap> Print(x->x);
function ( x ) return x; end
gap> fu := function(n) local a; a := n^2; if a < 100 then return a; else return 2*a; fi; end;
function( n ) ... end
gap> Print(fu);
function ( n ) local a; a := n ^ 2; if a < 100 then return a; else return 2 * a; fi; return; end

or, on a narrower screen

gap> Print(x->x);
function ( x ) return x; end
gap> fu := function(n) local a; a := n^2; if a < 100 then return a; else return 2*a; fi; end;
function( n ) ... end
gap> Print(fu);
function ( n ) local a; a := n ^ 2; if a < 100 then 
  return a; else return 2 * a; fi; return; end

is the nicer or more useful output.

I tend to prefer the first version (from which it is very easy to produce the more compact versions in an editor). To achieve that independent of the formatting status the code must be changed to include explicit spaces instead of line break hints after the explicit line breaks. I don't know how hard this would be to implement.

One could also argue that the output of function bodies should depend on the formatting status (version 1 above if the status is true and version 2 otherwise). But this is probably difficult because the functions which produce the output would then need to take the formatiing status of the "current?" output stream into account. Maybe we don't want such a dependency.

[ChrisJefferson]

I agree functions not being indented isn't something I want, but I always assumed that turning off PrintFormattingStatus was supposed to remove that indentation, the documentation mentions this option decides if functions are formatted "will be formatted with line breaks and indentation."

I do agree that over time there seems to be confusion about exactly what < and > are for (at least, I've been confused about it!)