RFC: Remove 'Stack trace' from break loop messages (???)

[fingolfin]

I've added this in PR #6257 but now am not so sure anymore I like it, and the way it is inserted. See this comment on PR #6416.

[ThomasBreuer]

I thought the format was inspired by that of the error messages in Julia, where the specific message text is followed by a line Stacktrace: (not Stack trace:) and then the description of the call stack, with numbers for the levels.

I have no preference for keeping the line in question or leaving it out.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.87%. Comparing base (8187e9e) to head (8dc2e93).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #6429   +/-   ##
=======================================
  Coverage   78.87%   78.87%           
=======================================
  Files         685      685           
  Lines      293550   293547    -3     
  Branches     8672     8672           
=======================================
  Hits       231530   231530           
+ Misses      60211    60209    -2     
+ Partials     1809     1808    -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[fingolfin]

Sorry, I was unclear: I am not necessarily saying we should merge this PR. I am just unsure if printing this line is good, bad or neutral. Personally I don't mind it an am also used to something like it from e.g. Julia. But I feel it kinda "slipped in under the radar" with PR #6257. So this is now an attempt to hear what others think about it: keep it? remove it again? or maybe adjust it?

[fingolfin]

Note also this implementation is a bit wonky. Perhaps Stack trace: should always be printed in Where?

[fingolfin]

Ah and this also breaks when certain packages are loaded: e.g. for me Browse is autoloaded, and therefore I never see the Stack trace::

gap> Display(OnBreak);
# /Users/mhorn/Projekte/GAP/gap/pkg/browse/lib/ncurses.gi:437
function (  )
    if not NCurses.isendwin(  ) then
        NCurses.endwin(  );
    fi;
    OnBreakSavedByBrowse(  );
    return;
end

[fingolfin]

See PR #6432 for an attempt to clean this up.

[fingolfin]

Here is an example of Stack trace: with an "empty" stack trace; this is what first lead to me questioning whether this line is helpful or not...

[fingolfin]

This is also addressed by PR #6432

[stertooy]

I prefer the message staying, at least when it's actually followed by a stack trace. Reason being that I think it might make errors clearer for users that have little programming experience.

If I didn't know what a stack trace was and I saw this:

Error, Assert: <cond> must be 'true' or 'false' (not the integer 1)
*[1] Assert( 0, 1 );
   @ *stdin*:44
<function "f">( <arguments> )

Then I'd probably be unsure whether this is all one big error message or not, and what to do with it. Whereas if I saw

Error, Assert: <cond> must be 'true' or 'false' (not the integer 1)
Stack trace:
*[1] Assert( 0, 1 );
   @ *stdin*:44
<function "f">( <arguments> )

I'd know that the first line is the actual error message, and everything below is something that's called a "stack trace", which I could then google.

[lgoettgens]

I am with @stertooy here: Keeping the heading would be great. And #6432 seems to fix the edge cases where it does not behave as wished for.