Project

General

Profile

Design #1698

indent: return a string?

Added by John Abbott almost 2 years ago. Updated over 1 year ago.

Status:
Closed
Priority:
Normal
Assignee:
Anna Maria Bigatti
Category:
enhancing/improving
Target version:
CoCoA-5.4.2
Start date:
27 Sep 2022
Due date:
% Done:

100%

Estimated time:
2.66 h
Spent time:
2.90 h

Description

To discuss:
Should indent return a string rather than print directly onto the standard output?

The main difference it would make to the user is that the value of It would be
overwritten by the string returned.

Related issues

Related to CoCoA-5 - Slug #750: C5 GUI: very slow when printing many short linesClosed2015-07-27

Related to CoCoA-5 - Bug #716: NotBuiltin.cpkg5: indent for MODULEIn Progress2015-05-20

History

#1 Updated by John Abbott almost 2 years ago

  • Related to Slug #750: C5 GUI: very slow when printing many short lines added

#2 Updated by John Abbott almost 2 years ago

  • Status changed from New to In Progress
  • % Done changed from 0 to 10

If we choose not to return a string then, for the benefit of #750, the
function could anyway print to an OpenOString object which is then
printed via a single call to println.

Note that the current impl of indent does not let us send the output
to anywhere other than standard output!

#3 Updated by John Abbott over 1 year ago

I have modified the code for indent (in NotBuiltin.cpkg5)
so that it returns a string.

The slow behaviour (issue #750) of the GUI does not arise with input indent(1..1000) -- using this new impl.

The problem with overwriting It can easily be avoided by calling print indent(VALUE);
Also we can now do print indent(VALUE) on OUTPUT;

Should I check in? [and revise the doc]

#4 Updated by John Abbott over 1 year ago

  • Assignee set to John Abbott

The CoCoA-5 tests do not pass with the new impl of indent.
The reason is simple: the revised impl puts in a newline everywhere where the old
impl put one (implicitly via println). The catch is that indent(L); is
handled the same as println indent(L); so indent appends a final newline,
and then println append another one.

Here is an example of the double newline:

/**/ L := [1,2];
/**/ indent(L);
[
  1,
  2
]

/**/

Possible options are:
  • (A) accept the new behaviour (and update the expected test outputs)
  • (B) modify indent so that it does not append a final newline
  • (C) something else... what?

With option (A) the problem would not be evident if the user does print indent(L);

JAA is undecided...

#5 Updated by Anna Maria Bigatti over 1 year ago

Possible options are:
  • (A) accept the new behaviour (and update the expected test outputs)
  • (B) modify indent so that it does not append a final newline
  • (C) something else... what?

I prefer (A).
The user can write "print indent(L);" ;-)
One nice effect of returning a string is that the user can also write "print indent(L) on F;" to print it on a file, an effect which I believe was not possibile before.

#6 Updated by Anna Maria Bigatti over 1 year ago

  • Related to Bug #716: NotBuiltin.cpkg5: indent for MODULE added

#7 Updated by John Abbott over 1 year ago

  • % Done changed from 10 to 50

I have changed the CoCoA-5 tests so that they do print indent(...); where previously there was just indent(...);
This leaves the output as it was, and perhaps has the advantage of illustrating the "correct" way of using indent.
OK?

Still need to update doc.

#8 Updated by John Abbott over 1 year ago

  • Status changed from In Progress to Feedback
  • % Done changed from 50 to 90

#9 Updated by Anna Maria Bigatti over 1 year ago

now that I'm using it, I'm having compatibility problems with the rest of the world still with earlier versions.
So I had an idea for "salvare capra e cavoli":
we call the new function IndentStr(X)
then

  define indent(X,n)
    print  IndentStr(X,n);
  enddefine;

and we have speed of writing on string + backward compatibility + full flexibility.

#10 Updated by Anna Maria Bigatti over 1 year ago

Anna Maria Bigatti wrote:

now that I'm using it, I'm having compatibility problems with the rest of the world still with earlier versions.
So I had an idea for "salvare capra e cavoli":
we call the new function IndentStr(X)
then
[...]
and we have speed of writing on string + backward compatibility + full flexibility.

implemented (sligthly more refined than above), and adjusted all tests. If OK I check it in.

#11 Updated by Anna Maria Bigatti over 1 year ago

It has to be said that having indent returning a string would be more compatibile with the functions format and latex (I need to think if there are other similar functions).
But I don't feel this is enough to break backward-compatibility if we can avoid it (and we can!).

#12 Updated by John Abbott over 1 year ago

Are these changes checked in?
[I think probably not yet]

#13 Updated by Anna Maria Bigatti over 1 year ago

John Abbott wrote:

Are these changes checked in?
[I think probably not yet]

yes, just now ;-)

#14 Updated by Anna Maria Bigatti over 1 year ago

  • Assignee changed from John Abbott to Anna Maria Bigatti

missing manual

#15 Updated by John Abbott over 1 year ago

I do not have IndentStr:

/**/ Starting("Inden");
[]
/**/ Starting("inden");
[record[IsExported := true, name := "$NotBuiltin.indent"]]

Can you check in? And write the manual (should be trivial)

#16 Updated by Anna Maria Bigatti over 1 year ago

John Abbott wrote:

I do not have IndentStr:
[...]

Can you check in? And write the manual (should be trivial)

Checked in (it was in NotBuiltin.cpkg5, but not exported).

Still to do: manual.

#17 Updated by John Abbott over 1 year ago

  • Status changed from Feedback to Closed
  • % Done changed from 90 to 100
  • Estimated time set to 2.66 h

I have just revised the manual entry for indent and IndentStr.
I think it makes sense for them to share an entry. Ooops. Must fix any xrefs.

#18 Updated by Anna Maria Bigatti over 1 year ago

fixed syntax IndentStr (rtn string) and modified example, also for level 2.
cvs-ed

Also available in: Atom PDF