-
Notifications
You must be signed in to change notification settings - Fork 17
Refactoring and fixing display issues #93
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
Merged
Merged
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
6919028
Format
maxlandon 1801e50
Merge branch 'master' into dev
maxlandon d1dba28
Fixes
maxlandon 6c6743c
chore: save current work before display refactor
maxlandon b735972
feat(display): refactor display engine for robust multiline rendering
maxlandon 2b1810a
fix(display): restore cursor positioning and fix upward line movement
maxlandon cbf1ab2
fix(display): integrate helpers and fix scrolling at screen bottom
maxlandon a0fa644
refactor(display): extract Refresh logic into helper methods
maxlandon 9a4429f
Ignore gemini context files
maxlandon 66a5446
Remove gemini
maxlandon File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| ## Gemini Added Memories | ||
| @instructions.md | ||
| @context.md |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| # Context for readline (~/code/github.com/reeflective/readline) | ||
|
|
||
| Add your project-specific context here. | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| # Fix Analysis: Multiline Display & Cursor Positioning | ||
|
||
|
|
||
| ## The Issue | ||
| The cursor jumps to the top of the input area and fails to return to the bottom during multiline editing, specifically when `multiline-column` options are disabled. | ||
|
|
||
| ## Analysis of `MultilineColumnPrint` | ||
| The function `MultilineColumnPrint` behaves differently based on configuration: | ||
| 1. **`multiline-column-numbered` (ON):** | ||
| * Iterates through all lines. | ||
| * Prints a string containing `\n` for each line. | ||
| * **Effect:** The cursor physically moves down `N` lines (where `N` is the number of logical lines). | ||
| 2. **`multiline-column` / Default (OFF):** | ||
| * The loop body or case is not entered (or returns empty string). | ||
| * **Effect:** The function prints nothing. The cursor does **not** move. | ||
|
|
||
| ## The Logic Flaw in `displayMultilinePrompts` (or `Refresh`) | ||
| The calling code typically looks like this: | ||
| ```go | ||
| if e.line.Lines() > 1 { | ||
| term.MoveCursorUp(e.lineRows) // Move to Top | ||
| e.prompt.MultilineColumnPrint() // Print Columns (Expectation: Moves Down) | ||
| // Missing: Explicit return to bottom if Print() didn't move us. | ||
| } | ||
| ``` | ||
|
|
||
| * **Scenario A (Numbered ON):** `MoveCursorUp` moves up. `Print` moves down (mostly). The cursor ends up near the bottom. The error is small (difference between logical newlines and wrapped rows). | ||
| * **Scenario B (All OFF):** `MoveCursorUp` moves up. `Print` does nothing. The cursor stays at the top. **This is the bug.** | ||
|
|
||
| ## Proposed Fix | ||
| The fix requires two adjustments: | ||
| 1. **Conditional Execution:** Only perform the "Move Up -> Print" sequence if a column mode is actually enabled. If disabled, there is no need to move up just to print nothing. | ||
| 2. **Cursor Correction:** When enabled, ensure the cursor returns to the correct bottom position by accounting for line wrapping. | ||
|
|
||
| **Algorithm:** | ||
| 1. Check if `multiline-column`, `numbered`, or `custom` is enabled. | ||
| 2. **If Enabled:** | ||
| * Move Cursor Up `e.lineRows`. | ||
| * Call `MultilineColumnPrint`. | ||
| * Move Cursor Down `e.lineRows - e.line.Lines()`. (Correction for wrapped lines vs logical newlines). | ||
| 3. **If Disabled:** | ||
| * Skip the block. The cursor remains at the bottom (where `displayLine` left it). | ||
|
|
||
| ## Secondary Prompts (`└ `) | ||
| The current logic only prints the Secondary Prompt (`PS2`) for the *current* (last) line. Previous lines display the column indicator (`|` or number). If columns are disabled, previous lines display nothing. This appears to be intended behavior, or at least separate from the cursor jump bug. | ||
|
|
||
| ``` | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
|
|
||
| ## Global Programming Instructions & Guidelines | ||
|
|
||
| - **Code removal:** Do not remove code that is not immediately relevant | ||
| to the changes you want to operate when being given a specific task. | ||
| - **Replacements:** When having to do many small (and rather or completely | ||
| identical) replacements in one or more files, perform these replacements | ||
| in a single call for each file, whenever possible. | ||
|
|
||
| ## Python Programming Guidelines | ||
|
|
||
| Here are some guidelines that I find useful when working on a Python codebase. | ||
|
|
||
| ### General Guidelines | ||
|
|
||
| * **Follow Project Conventions**: Adhere to the existing coding style, patterns, and practices used in the project. | ||
| * **Dependency Management**: Use the project's dependency manager (`uv`). | ||
|
|
||
| ### Style and Formatting | ||
|
|
||
| * **PEP 8**: Follow the [PEP 8](https://www.python.org/dev/peps/pep-0008/) style guide for Python code. Use tools like `black` for automatic formatting and `ruff` for linting to enforce it. | ||
| * **Docstrings**: Write clear and concise docstrings for all modules, functions, classes, and methods, following the [PEP 257](https://www.python.org/dev/peps/pep-0257/) conventions. Use a consistent format like Google Style or reStructuredText. | ||
| * **Typing**: Use Python's type hints (`str`, `int`, `List`, `Dict`) for all function signatures and variables where it improves clarity. Use a static type checker like `mypy` to verify type correctness. | ||
| * **Naming**: | ||
| * `snake_case` for functions, methods, and variables. | ||
| * `PascalCase` for classes. | ||
| * `UPPER_SNAKE_CASE` for constants. | ||
| * `_` for unused variables. | ||
|
|
||
| ### Code Organization | ||
|
|
||
| * **Modularity**: Break down large files into smaller, more manageable modules with a single responsibility. | ||
| * **Imports**: | ||
| * Import modules, not individual functions or classes, to avoid circular dependencies and naming conflicts (e.g., `import my_module` instead of `from my_module import my_function`). | ||
| * Group imports in the following order: | ||
| 1. Standard library imports (`os`, `sys`). | ||
| 2. Third-party library imports (`requests`, `pandas`). | ||
| 3. Local application/library specific imports. | ||
| * **Absolute vs. Relative Imports**: Prefer absolute imports (`from my_app.core import utils`) over relative imports (`from ..core import utils`) for clarity and to avoid ambiguity. | ||
|
|
||
| ### Testing | ||
|
|
||
| * **Unit Tests**: Write unit tests for all new code. Use a testing framework like `pytest`. | ||
| * **Test Coverage**: Aim for high test coverage, but focus on testing the logic and edge cases rather than just the line count. | ||
| * **Test Naming**: Name test files `test_*.py` and test functions `test_*()`. | ||
| * **Mocking**: Use mocking libraries like `unittest.mock` to isolate units of code and avoid external dependencies in tests. | ||
|
|
||
| ### Documentation | ||
|
|
||
| * **Comments**: Add comments to explain *why* something is done, not *what* is being done. The code itself should be self-explanatory. | ||
| * **Configuration**: Keep configuration separate from code. Use environment variables or configuration files (`.env`, `config.ini`, `settings.py`). | ||
|
|
||
| ### Best Practices | ||
|
|
||
| * **List Comprehensions**: Use list comprehensions for creating lists in a concise and readable way. | ||
| * **Generators**: Use generators and generator expressions for memory-efficient iteration over large datasets. | ||
| * **Error Handling**: Be specific in exception handling. Avoid bare `except:` clauses. | ||
| * **Context Managers**: Use the `with` statement when working with resources that need to be cleaned up (e.g., files, database connections). |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| # Display Engine Refactoring Plan | ||
|
|
||
| ## Goals | ||
| - Fix cursor positioning issues in multiline editing. | ||
| - Ensure robust rendering of prompts, input, and helpers. | ||
| - Standardize the display sequence to avoid "lost cursor" states. | ||
|
|
||
| ## Components | ||
| 1. **Prompts:** Primary (PS1), Secondary (PS2/`└ `), Multiline Columns (`│ `), Right/Tooltip. | ||
| 2. **Input:** Buffer text, Syntax Highlighting, Visual Selection, Auto-suggestions. | ||
| 3. **Helpers:** Hints, Completions. | ||
|
|
||
| ## Proposed Rendering Sequence (Refresh Cycle) | ||
|
|
||
| 1. **Preparation & Coordinates** | ||
| * Hide Cursor. | ||
| * Reset Cursor to start of input area (after Primary Prompt). | ||
| * Compute Coordinates: `StartPos`, `LineHeight`, `CursorPos` (row/col). | ||
| * Check for buffer height changes to clear potential artifacts below. | ||
|
|
||
| 2. **Primary Prompt** | ||
| * Reprint only if invalidated (e.g., transient prompt, clear screen). | ||
| * Otherwise, assume cursor starts at `StartPos`. | ||
|
|
||
| 3. **Input Area Rendering** | ||
| * **Input Line:** Print the full input buffer (highlighted + auto-suggestion). Cursor ends at the end of the input text. | ||
| * **Right Prompt:** | ||
| * Calculate position relative to the end of the input. | ||
| * Move cursor, print prompt, restore cursor to end of input. | ||
| * **Multiline Indicators (Columns/Secondary):** | ||
| * Iterate through input lines. | ||
| * Move to start of each line. | ||
| * Print column indicators (`│ `, numbers) and secondary prompts (`└ `). | ||
| * **Crucial:** Return cursor to the **end of the input text** after this pass. | ||
|
|
||
| 4. **Helpers Rendering** | ||
| * Move cursor below the last line of input. | ||
| * **Hints:** Print hints. | ||
| * **Completions:** Print completion menu/grid. | ||
| * **Cleanup:** Clear any remaining lines below if the new render is shorter. | ||
|
|
||
| 5. **Final Cursor Positioning** | ||
| * Move cursor from bottom of helpers -> End of Input. | ||
| * Move cursor from End of Input -> Actual Cursor Position (using `CursorPos`). | ||
| * Show Cursor. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| # Temporary Notes for readline (~/code/github.com/reeflective/readline) | ||
|
|
||
| This file is for your private notes, prompts, and scratchpad content. | ||
| It is not directly sent to Gemini, but you can easily copy/paste from here. | ||
|
|
||
| The behavior in the shell is the following: | ||
|
|
||
| I enter a first line: `testing \` | ||
| I press enter, and the shell correctly goes to a new line. | ||
| I then type again: `testing \` | ||
| and the prompt goes back to its initial position on the first line (which means it goes one up one line too much. | ||
| Starting from there, everytime I enter a character (thus causing the shell to redisplay the line) the cursor goes up 2 lines, while it should not. | ||
|
|
||
| Based on this, I want you to tell me what do you suspect in the code is not working correctly. | ||
|
|
||
| Investigate the codebase and identify the issue. | ||
|
|
||
| Consider this snippet from the codebase. | ||
|
|
||
| helpersMoved := e.displayHelpers() | ||
| if helpersMoved { | ||
| e.cursorHintToLineStart() | ||
| e.lineStartToCursorPos() | ||
| } else { | ||
| e.lineEndToCursorPos() | ||
| } | ||
|
|
||
|
|
||
| I'm pretty sure the problem is in this snippet. | ||
| I suspect that cursorHintToLineStart() or lineStartToCursorPos() are miscalculating something when | ||
| the line is a multiline string. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| # Master To-Do List for readline (~/code/github.com/reeflective/readline) | ||
|
|
||
| ## Child To-Do Lists | ||
| (BEGIN AUTO-GENERATED-TODOS) | ||
| (END AUTO-GENERATED-TODOS) |
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
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
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.