fyne-io / terminal

A graphical terminal emulator for Linux using Fyne
Other
256 stars 38 forks source link

Add escapeInsertCharacters function to handle '@' character #66

Closed figuerom16 closed 10 months ago

figuerom16 commented 10 months ago

This allows for character inserting which works great as 1@ escapes were not being handled.

This did reveal another issue where 1P escapeDeleteChars is working, but the terminal isn't refreshing ( t.content.Refresh() and t.Refresh() does not work) upon delete leaving blank spaces. Resizing the terminal causes characters to be displayed correctly.

Is there a way to manually update the display?

andydotxyz commented 10 months ago

but the terminal isn't refreshing (t.content.Refresh() and t.Refresh() does not work)

Asking the TextGrid to refresh certainly should refresh the display. I think you'll need to look into why the output and data are not matching when it's Refresh is called rather than looking for a way around it.

figuerom16 commented 10 months ago

I found the issue. I couldn't find any flaws in widget.TextGrid. Inspecting escapeDeleteChars more closely was showing that completely blank cells were being appended which is not desired. It explains why a resize cleared those cells out.

escape(Insert/Delete)Chars are now working nicely.

mgazza commented 10 months ago

Not to jump in but can we please try and add tests ideally the PR would start with a failing test and then the commit that fixes the test case.

figuerom16 commented 10 months ago

Not to jump in but can we please try and add tests ideally the PR would start with a failing test and then the commit that fixes the test case.

You're right. I'll remember to create the test first next time to show what's being accomplished.

This fixes the overwriting text issue and delete not rendering correctly. When altering a command with spaces the terminal would switch to Insert/Delete (ICH/DCH) escapes when changing a word that is not the last word.

andydotxyz commented 10 months ago

FYI as a test case you can see this fixing the output of sl command - that's made my day!

figuerom16 commented 10 months ago

@andydotxyz That is a really fun test, sl, I've been using Linux for 15+ years and didn't know about that. I did notice that some of the train isn't rendering; some of the horizontal lines. I'll look into that next.

andydotxyz commented 10 months ago

I think that relates to the thickness of the font's underline/dash and the resolution of your display. Try testing with FYNE_SCALE=1.5 and see if they become visible - I noticed them missing on one of my machines but not the other.

figuerom16 commented 10 months ago

That fixed it. I'll have to remember that trick. 1080p monitor.

andydotxyz commented 10 months ago

That fixed it. I'll have to remember that trick. 1080p monitor.

I think really we need to figure how to fix it so that lines don't disappear at lower resolutions - but that is a different story!

figuerom16 commented 10 months ago

Increasing fontSize from 12 to 14 fixed it for me under cmd/fyneterm/theme.go. Unthemed terminals don't seem to have any issues. No FYNE_SCALE adjustment.

func newTermTheme() *termTheme {
    return &termTheme{Theme: fyne.CurrentApp().Settings().Theme(), fontSize: 14}
}
andydotxyz commented 10 months ago

Yup, that would be the same as increasing the FYNE_SCALE (though it impacts only text and not everything). Run with font size 14 then apply FYNE_SCALE=0.8 and I guess they will disappear again?

figuerom16 commented 10 months ago

Yup 14 fontSize and FYNE_SCALE=0.8 causes the horizontal lines to disappear again.