REPL History + Color formatting#19
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds interactive line editing and colored output formatting to the REPL. Two new units introduce a UNIX-terminal line editor with history support and a value formatter with ANSI color codes. The main REPL program is updated to integrate these components, replacing basic output with formatted results. Changes
Sequence DiagramsequenceDiagram
participant User
participant REPL as REPL.dpr
participant Editor as TLineEditor
participant Terminal
participant Formatter as FormatREPLValue
participant Engine as Goccia Engine
loop REPL Session
REPL->>Editor: ReadLine(prompt)
Editor->>Terminal: Enter raw mode
Editor->>Terminal: Display prompt
User->>Terminal: Input characters
Terminal->>Editor: Read character
Editor->>Terminal: Redraw line (echo, cursor control)
Editor->>Terminal: Handle history navigation (up/down arrows)
Note over Editor: Store in history on Enter/Return
Editor->>Terminal: Exit raw mode
Editor-->>REPL: Return TLineReadResult (lrLine/lrExit)
alt Input received (lrLine)
REPL->>Engine: Execute source code
Engine-->>REPL: Return TGocciaValue
REPL->>Formatter: FormatREPLValue(value)
Formatter-->>REPL: Colored string output
REPL->>Terminal: Display formatted result
else User exit (lrExit)
REPL->>REPL: Break loop & cleanup
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes The review requires careful attention to the UNIX terminal handling logic in the line editor (raw mode, character reading, cursor positioning), history navigation implementation, and state management. The formatter logic is straightforward, but the line editor's interactive behavior and terminal-specific code paths (guarded by Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Benchmark Results134 benchmarks · 134 unchanged · avg -1.6% arrays.js — 11 unchanged · avg -1.3%
classes.js — 10 unchanged · avg -2.6%
closures.js — 11 unchanged · avg -1.0%
collections.js — 12 unchanged · avg -0.6%
destructuring.js — 14 unchanged · avg -1.8%
fibonacci.js — 3 unchanged · avg -0.9%
json.js — 11 unchanged · avg -2.0%
jsx.jsx — 21 unchanged · avg -2.1%
numbers.js — 11 unchanged · avg -0.2%
objects.js — 7 unchanged · avg -2.4%
promises.js — 12 unchanged · avg -3.1%
strings.js — 11 unchanged · avg -0.7%
Measured on ubuntu-latest x64. Changes within ±7% are considered insignificant. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
units/Goccia.REPL.LineEditor.pas (1)
21-22: MarkACursorPosasconst.
The parameter isn’t modified. As per coding guidelines: Preferconstparameters for all parameters that are not modified in the function body.♻️ Suggested update
- procedure RedrawLine(const APrompt, ABuffer: string; ACursorPos: Integer); + procedure RedrawLine(const APrompt, ABuffer: string; const ACursorPos: Integer);-procedure TLineEditor.RedrawLine(const APrompt, ABuffer: string; ACursorPos: Integer); +procedure TLineEditor.RedrawLine(const APrompt, ABuffer: string; const ACursorPos: Integer);Also applies to: 80-81
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@units/Goccia.REPL.LineEditor.pas` around lines 21 - 22, Change the ACursorPos parameter to a const parameter wherever it’s declared and implemented (e.g., procedure RedrawLine(const APrompt, ABuffer: string; const ACursorPos: Integer) and its implementation), and do the same for the other routine(s) that accept ACursorPos (the occurrence mentioned around lines 80-81); make sure the interface and implementation signatures match and update any forward declarations so all ACursorPos parameters are declared as const.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@units/Goccia.REPL.LineEditor.pas`:
- Around line 10-29: Rename the class TLineEditor to TGocciaLineEditor and
update all corresponding declarations and references: change the class
declaration name to TGocciaLineEditor, update every
method/constructor/destructor definition from "procedure/function
TLineEditor.XXX" to "procedure/function TGocciaLineEditor.XXX" (including
Create, Destroy, ReadLine, AddToHistory, SetRawMode, RestoreTerminal, ReadChar,
RedrawLine, ReadLineRaw), and update the variable declaration and instantiation
in REPL.dpr to use TGocciaLineEditor instead of TLineEditor so all types and
calls match the new TGoccia<Name> prefix.
---
Nitpick comments:
In `@units/Goccia.REPL.LineEditor.pas`:
- Around line 21-22: Change the ACursorPos parameter to a const parameter
wherever it’s declared and implemented (e.g., procedure RedrawLine(const
APrompt, ABuffer: string; const ACursorPos: Integer) and its implementation),
and do the same for the other routine(s) that accept ACursorPos (the occurrence
mentioned around lines 80-81); make sure the interface and implementation
signatures match and update any forward declarations so all ACursorPos
parameters are declared as const.
Comment on lines
+10
to
+29
| type | ||
| TLineReadResult = (lrLine, lrExit); | ||
|
|
||
| TLineEditor = class | ||
| private | ||
| FHistory: TStringList; | ||
| FHistoryIndex: Integer; | ||
| {$IFDEF UNIX} | ||
| procedure SetRawMode; | ||
| procedure RestoreTerminal; | ||
| function ReadChar: Char; | ||
| procedure RedrawLine(const APrompt, ABuffer: string; ACursorPos: Integer); | ||
| function ReadLineRaw(const APrompt: string; out ALine: string): TLineReadResult; | ||
| {$ENDIF} | ||
| public | ||
| constructor Create; | ||
| destructor Destroy; override; | ||
| function ReadLine(const APrompt: string; out ALine: string): TLineReadResult; | ||
| procedure AddToHistory(const ALine: string); | ||
| end; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n --type=pas --type=dpr "TLineEditor" -C 2Repository: frostney/GocciaScript
Length of output: 92
🏁 Script executed:
rg -n "TLineEditor" --glob="*.pas" --glob="*.dpr" -C 2Repository: frostney/GocciaScript
Length of output: 3203
Rename TLineEditor to TGocciaLineEditor.
Class names must use the TGoccia<Name> prefix. Update the class declaration in units/Goccia.REPL.LineEditor.pas (line 13), all method/constructor/destructor definitions within that file (lines 45, 62, 68, 80, 93, 274, 280, 286, 297), and the variable declaration and instantiation in REPL.dpr (lines 24, 39).
♻️ Suggested rename (apply consistently)
- TLineEditor = class
+ TGocciaLineEditor = classAll method definitions must be updated from procedure TLineEditor.MethodName to procedure TGocciaLineEditor.MethodName, and so on.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| type | |
| TLineReadResult = (lrLine, lrExit); | |
| TLineEditor = class | |
| private | |
| FHistory: TStringList; | |
| FHistoryIndex: Integer; | |
| {$IFDEF UNIX} | |
| procedure SetRawMode; | |
| procedure RestoreTerminal; | |
| function ReadChar: Char; | |
| procedure RedrawLine(const APrompt, ABuffer: string; ACursorPos: Integer); | |
| function ReadLineRaw(const APrompt: string; out ALine: string): TLineReadResult; | |
| {$ENDIF} | |
| public | |
| constructor Create; | |
| destructor Destroy; override; | |
| function ReadLine(const APrompt: string; out ALine: string): TLineReadResult; | |
| procedure AddToHistory(const ALine: string); | |
| end; | |
| type | |
| TLineReadResult = (lrLine, lrExit); | |
| TGocciaLineEditor = class | |
| private | |
| FHistory: TStringList; | |
| FHistoryIndex: Integer; | |
| {$IFDEF UNIX} | |
| procedure SetRawMode; | |
| procedure RestoreTerminal; | |
| function ReadChar: Char; | |
| procedure RedrawLine(const APrompt, ABuffer: string; ACursorPos: Integer); | |
| function ReadLineRaw(const APrompt: string; out ALine: string): TLineReadResult; | |
| {$ENDIF} | |
| public | |
| constructor Create; | |
| destructor Destroy; override; | |
| function ReadLine(const APrompt: string; out ALine: string): TLineReadResult; | |
| procedure AddToHistory(const ALine: string); | |
| end; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@units/Goccia.REPL.LineEditor.pas` around lines 10 - 29, Rename the class
TLineEditor to TGocciaLineEditor and update all corresponding declarations and
references: change the class declaration name to TGocciaLineEditor, update every
method/constructor/destructor definition from "procedure/function
TLineEditor.XXX" to "procedure/function TGocciaLineEditor.XXX" (including
Create, Destroy, ReadLine, AddToHistory, SetRawMode, RestoreTerminal, ReadChar,
RedrawLine, ReadLineRaw), and update the variable declaration and instantiation
in REPL.dpr to use TGocciaLineEditor instead of TLineEditor so all types and
calls match the new TGoccia<Name> prefix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit