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

fix ansi pager history santizer #1657

Closed

Conversation

nerdrew
Copy link
Contributor

@nerdrew nerdrew commented May 28, 2019

fixes #1642

kitty/window.py Outdated
@@ -123,7 +123,7 @@ def remove_sgr(line):
def remove_both(line):
return pat.sub('', line.replace('\r', ''))

if as_ansi:
if not as_ansi:
return remove_both if add_wrap_markers else remove_sgr
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't tell if the logic for add_wrap_markers also needs to change.

@nerdrew nerdrew force-pushed the fix-pager-history-ansi-sanitizer branch from 86e507e to de265c8 Compare May 28, 2019 19:05
@nerdrew nerdrew force-pushed the fix-pager-history-ansi-sanitizer branch from de265c8 to 760e23e Compare May 28, 2019 19:22
@nerdrew
Copy link
Contributor Author

nerdrew commented May 29, 2019

The pager wrap still has ^M characters in it for me. The stdin = pipe_data['text'] change fixed it for me, though I'm not sure what deleting stdin = stdin.encode('utf-8') will break.

Here are screen shots of the various states for me. They all use the following invocation:

python3 . -o "map=f1 pipe @text_wrap overlay less -R -n" -o "scrollback_lines 50" -o "map=f2 pipe @ansi_wrap overlay less -R -n" -o "map=f3 pipe @text overlay less -R -n"

And this command to create the scrollback:

echo aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa; for i in $(seq 0 100); echo $i

Regular show_scrollback (looks correct):
Screen Shot 2019-05-28 at 21 15 44

pipe @text_wrap overlay less -R -n (has spurious ^M characters and text is not wrapped, less does the wrapping):
Screen Shot 2019-05-28 at 21 16 15

pipe @ansi_wrap overlay less -R -n (color looks right, but again has spurious ^M characters and text is not wrapped, less does the wrapping):
Screen Shot 2019-05-28 at 21 17 13

pipe @text overlay less -R -n (looks right):
Screen Shot 2019-05-28 at 21 17 24

@kovidgoyal
Copy link
Owner

kovidgoyal commented May 29, 2019

As noted here: https://sw.kovidgoyal.net/kitty/pipe.html

adding _wrap means that the output will have carriage returns in it. This is meant for programs that want to do wrapping themselves, typically when the screen they are writing to is of different size than the kitty window. If you want to pie to less in an overlay, dont use _wrap just use @ansi and you should be fine. There wil be no wrap markers causing less to do the wrapping itself, which should exactly match the kitty window, since the overlay window has the same size.

@nerdrew
Copy link
Contributor Author

nerdrew commented May 29, 2019

Ah! I missed that part of the documentation. That totally makes sense. Thanks for the awesome software!

@nerdrew nerdrew deleted the fix-pager-history-ansi-sanitizer branch May 29, 2019 04:27
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.

The formatting for map f1 pipe @ansi overlay less +G -R is off on macos
2 participants