Skip to content
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

Add sections to BlueJ terminal to divide up different method calls #2312

Merged
merged 9 commits into from
Jan 24, 2024
27 changes: 25 additions & 2 deletions bluej/src/main/java/bluej/terminal/Terminal.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ of the License, or (at your option) any later version.
import bluej.utility.Utility;
import bluej.utility.javafx.JavaFXUtil;
import javafx.application.Platform;
import javafx.beans.binding.Bindings;
import javafx.beans.property.BooleanProperty;
import javafx.beans.property.ReadOnlyDoubleWrapper;
import javafx.beans.property.ReadOnlyStringWrapper;
Expand Down Expand Up @@ -89,6 +88,7 @@ of the License, or (at your option) any later version.
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

Expand Down Expand Up @@ -143,7 +143,7 @@ public final class Terminal
private final BooleanProperty showingProperty = new SimpleBooleanProperty(false);

@OnThread(Tag.Any) private final Reader in = new TerminalReader();
@OnThread(Tag.Any) private final Writer out = new TerminalWriter(false);
@OnThread(Tag.Any) private final TerminalWriter out = new TerminalWriter(false);
@OnThread(Tag.Any) private final Writer err = new TerminalWriter(true);

private Stage window;
Expand Down Expand Up @@ -177,6 +177,7 @@ else if (errorText != null)
}
};
text.getStyleClass().add("terminal-output");
text.styleProperty().bind(PrefMgr.getEditorFontCSS(PrefMgr.FontCSS.EDITOR_SIZE_AND_FAMILY));
text.addSelectionListener((caret, anchor) -> {
if (errorText != null && errorText.getCaretEditorPosition().getPosition() != errorText.getAnchorEditorPosition().getPosition())
{
Expand Down Expand Up @@ -509,6 +510,7 @@ private void methodCall(String callString)
if(clearOnMethodCall.get()) {
clear();
}
text.markNewSection();
if(recordMethodCalls.get()) {
text.append(new StyledSegment(STDOUT_METHOD_RECORDING, callString + "\n"));
}
Expand Down Expand Up @@ -724,6 +726,24 @@ public void blueJEvent(int eventId, Object arg, Project prj)
}
else if (eventId == BlueJEvent.EXECUTION_RESULT) {
methodResult((ExecutionEvent) arg);
endSectionWhenNoPendingWrites();
}
}

/**
* End the output section, if there are no writes pending to stdout.
* Otherwise, reschedule ourselves (to end the section once no more writes are pending)
*/
@OnThread(Tag.FXPlatform)
private void endSectionWhenNoPendingWrites()
{
if (out.pendingWrites.get() > 0)
{
JavaFXUtil.runAfterCurrent(() -> endSectionWhenNoPendingWrites());
}
else
{
text.endSection();
}
}

Expand Down Expand Up @@ -889,6 +909,7 @@ public void close() { }
private class TerminalWriter extends Writer
{
private boolean isErrorOut;
private AtomicInteger pendingWrites = new AtomicInteger(0);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although the name seems clear, I'd suggest to add a comment to describe the nature of this variable as when looking at its type only, it might not be obvious what it "counts".


TerminalWriter(boolean isError)
{
Expand All @@ -899,6 +920,7 @@ private class TerminalWriter extends Writer
public void write(final char[] cbuf, final int off, final int len)
{
try {
pendingWrites.incrementAndGet();
// We use a wait so that terminal output is limited to
// the processing speed of the event queue. This means the UI
// will still respond to user input even if the output is really
Expand All @@ -922,6 +944,7 @@ public void write(final char[] cbuf, final int off, final int len)
finally
{
written.complete(true);
pendingWrites.decrementAndGet();
}
});
// Timeout in case something goes wrong with the printing:
Expand Down
193 changes: 192 additions & 1 deletion bluej/src/main/java/bluej/terminal/TerminalTextPane.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,15 @@ of the License, or (at your option) any later version.
package bluej.terminal;

import bluej.Config;
import bluej.editor.base.BackgroundItem;
import bluej.editor.base.BaseEditorPane;
import bluej.editor.base.EditorPosition;
import bluej.editor.base.TextLine.StyledSegment;
import bluej.utility.javafx.FXPlatformRunnable;
import bluej.utility.javafx.JavaFXUtil;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import javafx.geometry.Insets;
import javafx.scene.Cursor;
import javafx.scene.control.ContextMenu;
import javafx.scene.input.Clipboard;
Expand All @@ -37,12 +39,18 @@ of the License, or (at your option) any later version.
import javafx.scene.input.MouseButton;
import javafx.scene.input.MouseEvent;
import javafx.scene.input.ScrollEvent;
import javafx.scene.layout.BackgroundFill;
import javafx.scene.layout.CornerRadii;
import javafx.scene.paint.Color;

import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.List;
import java.util.ListIterator;
import java.util.Map;
import java.util.Optional;
import java.util.stream.Collectors;

/**
Expand All @@ -65,6 +73,99 @@ public abstract class TerminalTextPane extends BaseEditorPane
private Pos caretPos = new Pos(0, 0, 0);
private Pos anchorPos = new Pos(0, 0, 0);

// The line may be negative in some circumstances (see Section, below).
// In this case, column should be ignored. If the column is Integer.MAX_VALUE
// it means to take the whole line as included, no matter how long.
// Both column and line are zero-based.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand the comment but I don't see how it applies to the method in the record.

record TerminalPos(int line, int column)
{
public TerminalPos subtractLines(int linesToSubtract)
{
return new TerminalPos(line - linesToSubtract, column);
}
}

// endLine is negative if ongoing. startLine is negative if the start has scrolled off the top
// It is possible for them to both be negative if the section is very long and ongoing.
// All values of the pos are inclusive. The columns should be ignored if the line is negative.
record Section(TerminalPos start, TerminalPos end) {}

private final ArrayList<Section> currentSections = new ArrayList<>();

// Get the current end position of the content as a start position
// This is different to getCurEnd() because it does not do any extra
// calculation about trailing newlines.
private TerminalPos getCurStart()
{
if (content.isEmpty())
{
return new TerminalPos(0, 0);
}
else
{
return new TerminalPos(content.size() - 1, content.get(content.size() - 1).getText().length());
}
}

// Get the current end position of the content as an end position
// This is different to getCurStart() because if the content ends if a newline
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small typo near the end of the line "if a newline" --> "is a newline"

// (the last content is a blank line), we take the end position as being the end
// of the previous line, not the start of the new line (if there's no content on that line)
private TerminalPos getCurEnd()
{
// If the final line is empty, we count the current end as the whole of the line before
if (content.isEmpty())
{
return new TerminalPos(0, 0);
}
else
{
String lastLine = content.get(content.size() - 1).getText();
if (lastLine.isEmpty())
{
return new TerminalPos(content.size() - 2, Integer.MAX_VALUE);
}
else
{
return new TerminalPos(content.size() - 1, lastLine.length());
}
}
}

public void markNewSection()
{
if (!currentSections.isEmpty())
{
int lastLineIndex = currentSections.size() - 1;
// If current last section was marked as ongoing, finish it:
if (currentSections.get(lastLineIndex).end.line < 0)
{
currentSections.set(lastLineIndex, new Section(currentSections.get(lastLineIndex).start, getCurEnd()));
}
}
currentSections.add(new Section(getCurStart(), new TerminalPos(-1, 0)));
}

// End the current section of content.
public void endSection()
{
if (!currentSections.isEmpty())
{
// If the content is empty we get rid of all sections:
if (content.isEmpty())
{
currentSections.clear();
}
else
{
int lastSection = currentSections.size() - 1;
currentSections.set(lastSection, new Section(currentSections.get(lastSection).start, getCurEnd()));
updateRender(false);
}
}
}


public TerminalTextPane()
{
super(false, new BaseEditorPaneListener()
Expand Down Expand Up @@ -210,7 +311,82 @@ protected void keyPressed(KeyEvent event)

public abstract void focusPrevious();
public abstract void focusNext();


@Override
protected void updateRender(boolean ensureCaretVisible)
{
super.updateRender(ensureCaretVisible);
// Recalculate all sections in the terminal:
HashMap<Integer, List<BackgroundItem>> map = new HashMap<>();
boolean reschedule = false;
for (int i = 0; i < content.size(); i++)
{
for (Section s : currentSections)
{
final double singleRadius = 5;
// All are specific to this section, on this line:
double topRadius = 0, bottomRadius = 0;
// Top or bottom inset of 0 basically means "don't draw the grey line":
double topInset = 0, bottomInset = 0;
// Default is whole width:
double leftInset = 0, rightInset = getTextDisplayWidth()-1.0;

// Each section could begin and/or end on the current line
// If neither, it may be ongoing through this line, or just not overlapping at all.
// So there's quite a few circumstances to consider. We start with beginning:
if (s.start.line == i)
{
topRadius = singleRadius;
topInset = 1;
if (s.start.column >= 0 && s.start.column < content.get(i).getText().length())
{
Optional<Double> edge = lineDisplay.calculateLeftEdgeX(i, s.start.column);
reschedule |= edge.isEmpty();
leftInset = edge.orElse(leftInset);
}
if (s.end.line == i)
{
bottomRadius = singleRadius;
bottomInset = 1;
if (s.end.column >= 0 && s.end.column <= content.get(i).getText().length())
{
Optional<Double> edge = lineDisplay.calculateLeftEdgeX(i, s.end.column);
reschedule |= edge.isEmpty();
rightInset = edge.orElse(rightInset);
}
}
}
else if (s.end.line == i)
{
bottomRadius = singleRadius;
bottomInset = 1;
if (s.end.column >= 0 && s.end.column <= content.get(i).getText().length())
{
Optional<Double> edge = lineDisplay.calculateLeftEdgeX(i, s.end.column);
reschedule |= edge.isEmpty();
rightInset = edge.orElse(rightInset);
}
}
else if (!(s.start.line < i && (s.end.line == -1 || s.end.line > i)))
{
// Does not overlap this line at all:
continue;
}

CornerRadii radii = new CornerRadii(topRadius, topRadius, bottomRadius, bottomRadius, false);
Insets bodyInsets = new Insets(topInset, 1, bottomInset, 1);
map.computeIfAbsent(i, _i -> new ArrayList<>()).add(new BackgroundItem(leftInset, rightInset - leftInset,
new BackgroundFill(Color.LIGHTGRAY, radii, null),
new BackgroundFill(Color.WHITE, radii, bodyInsets)));
}
}
lineDisplay.applyScopeBackgrounds(map);
if (reschedule)
{
JavaFXUtil.runAfterNextLayout(getScene(), () -> updateRender(false));
}
}

public final void requestFocusAndShowCaret()
{
requestFocus();
Expand Down Expand Up @@ -318,6 +494,20 @@ public void trimToMostRecentNLines(int numLines)
anchorPos = makePosition(
newAnchorLine, Math.min(anchorPos.getColumn(), getLineLength(newAnchorLine))
);
// Adjust line offset of any current lines in the display to match what we've just changed:
for (ListIterator<Section> iterator = currentSections.listIterator(); iterator.hasNext(); )
{
Section s = iterator.next();
// Check for scrolling off the top entirely, if it's not currently ongoing:
if (s.end.line > 0 && s.end.line < linesToSubtract)
{
iterator.remove();
}
else
{
iterator.set(new Section(s.start.subtractLines(linesToSubtract), s.end.subtractLines(linesToSubtract)));
}
}
updateRender(false);
}
}
Expand Down Expand Up @@ -354,6 +544,7 @@ public void clear()
caretPos = new Pos(0, 0, 0);
anchorPos = new Pos(0, 0, 0);
setContent(Collections.singletonList(new ContentLine(new ArrayList<>())));
currentSections.clear();
}

/**
Expand Down