Skip to content

Conversation

@aarondfrancis
Copy link
Collaborator

No description provided.

@aarondfrancis
Copy link
Collaborator Author

Fixed a lot of the issues but still have a few cursor movement problems with Ngrok. Will need to capture some Ngrok output and try to make the emulator render it faithfully

@aarondfrancis
Copy link
Collaborator Author

This is not ready yet. There are still several failing tests. I think we'll have to switch to using arrays as the character buffer instead of strings.

@CamKem
Copy link
Contributor

CamKem commented Feb 25, 2025

@aarondfrancis - Just pulled these latest changes; you're making some good ground on the fix.
I did however, noticed a couple of small things while putting it through its paces.

You may already be aware, either way I'll document here, in case your not & want to work on it this branch. Otherwise, I'm happy to add them in as seperate issues to be looked at once this fix is merged and we can work on it afterwards.

  1. If you start / stop solo several times, occasionally on startup the About command loads pro scrolled 2 rows up.
Screenshot 2025-02-25 at 4 35 11 PM
  1. On the heart & sheild emoji screens (2 of the 5) rendered by ngrok, the emoji and other text is malformed. Additionally rarely on render, left shifted line visibly not lining up on the right side too, but it's not readily repeatable.
Screenshot 2025-02-25 at 4 26 42 PM

@CamKem CamKem mentioned this pull request Feb 25, 2025
2 tasks
@aarondfrancis
Copy link
Collaborator Author

@CamKem keep em coming! I think I've almost cracked it. I'll add tests for the "about" thing, and I've noticed one or two other line issues, so I miiiiight be able to isolate it

@aarondfrancis
Copy link
Collaborator Author

@CamKem I pulled part of your backspace fix from #72.

I added a few tests for backspace and delete, but weirdly the tests only pass without the replacement of the delete character. I'm not entirely sure what the correct behavior should be, but this matches the iTerm implementation.

You can see the diff here: 10a9e32

@aarondfrancis aarondfrancis marked this pull request as ready for review February 27, 2025 16:36
@aarondfrancis
Copy link
Collaborator Author

@CamKem you wanna put this through its paces before I merge it in? I think I fixed most of the issues (famous last words)

@CamKem
Copy link
Contributor

CamKem commented Feb 28, 2025

@CamKem you wanna put this through its paces before I merge it in? I think I fixed most of the issues (famous last words)

@aarondfrancis looks pretty good, however still a couple of issues persisting. Honestly they maybe edge cases that the time invested would outweigh the benefit gained at this point, i'll leave that up to you. Alternatively we can add these as seperate issues to be worked on after this is merged, so you can tag a more stable release with the work you have done.

  1. If you run & quit the solo multiple times, on occasion the about command will render with the top line not being rendered.
Screenshot 2025-02-28 at 10 49 55 AM
  1. When the terminal is smallish (but not small enough to cause line breaks), there is a slight rendering issue with the ascii logo art (early break?)
Screenshot 2025-02-28 at 10 22 44 AM
  1. Grrr ngrok! There is persistent issues with the ❤️ & 🛡️ emojis which represent 2 out of a possible 5 screens that ngrok can render.
Screenshot 2025-02-28 at 11 12 12 AM
  1. Sometimes the top line of the ngrok screen doesn't complete the background row.
Screenshot 2025-02-28 at 11 10 08 AM

@CamKem
Copy link
Contributor

CamKem commented Feb 28, 2025

@CamKem I pulled part of your backspace fix from #72.

I added a few tests for backspace and delete, but weirdly the tests only pass without the replacement of the delete character. I'm not entirely sure what the correct behavior should be, but this matches the iTerm implementation.

You can see the diff here: 10a9e32

@aarondfrancis - Interesting, I went off the VT100 / GNUReadline key mappings (https://www.gnu.org/software/bash/manual/html_node/Readline-Bare-Essentials.html) which should work as the screen is a VT100 emulator using the GNUReadline key mappings.

Thanks for the heads up, can look further into it on my PR if need be.

@aarondfrancis aarondfrancis merged commit 697679a into main Mar 19, 2025
13 checks passed
@aarondfrancis aarondfrancis deleted the af-fix-ngrok branch March 19, 2025 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants