yurikhan / kitty_grab

Keyboard-driven screen grabber for Kitty
GNU General Public License v3.0
178 stars 12 forks source link

select stream adds newline on display linebreak #19

Open omgold opened 2 years ago

omgold commented 2 years ago

When I do stream selection (shift + arrows with default bindings) on a output line which is displayed on the terminal in multiple lines (due to being too long) I get a newline inserted at the point where the display breaks the line. This doesn't happen when I select using the mouse.

yurikhan commented 2 years ago

I should research that sometime. Or if you know how to fix it, a pull request will be welcome.

omgold commented 2 years ago

I did take a look. The origin of the issue is this

    content = window.as_text(as_ansi=True, add_history=True,
                             add_wrap_markers=True)

Using add_wrap_markers is what gives you the undesired line breaks. Unfortunately a just disabling it will just make the long lines appear truncated. Couldn't figure out quickly why and how to fix that.

yurikhan commented 2 years ago

Yeah, I guess it was either the newlines or having to implement wrapping. This needs further research.

omgold commented 2 years ago

An (imperfect) workaround would be to assume there should be no line break when the line has as many characters as the window width:

diff --git a/_grab_ui.py b/_grab_ui.py
index f58f56a..031cbb5 100644
--- a/_grab_ui.py
+++ b/_grab_ui.py
@@ -616,9 +616,10 @@ class GrabHandler(Handler):
         self._select(direction, self.region_types[region_type])

     def confirm(self, *args: Any) -> None:
+        screen_cols = self.screen_size.cols
         start, end = self._start_end()
-        self.result = {'copy': '\n'.join(
-            line_slice
+        self.result = {'copy': ''.join(
+            (line_slice + "\n" if len(line_slice)<screen_cols else line_slice)
             for line in range(start.line, end.line + 1)
             for plain in [unstyled(self.lines[line - 1])]
             for start_x, end_x in [self.mark_type.selection_in_line(
yurikhan commented 2 years ago

No, that won’t fly at all. In Midnight Commander and many other full-screen terminal applications, each line has as many characters as the window width.

Also, some characters occupy more than one cell, and some sequences of more than one character occupy a single cell, so you’re risking both false positives and false negatives.

omgold commented 2 years ago

I'm aware. The issue of unicode combiners and wide characters can't be fixed so easily, I guess. About curses windowed interfaces like mc, I wouldn't particularly mind. Don't know why one would want to do a multi-line selection in mc.

omgold commented 2 years ago

Another (rather insane) method would be to fetch 2 copies of the contents from kitty, one with extra line breaks, one without, and then reconstruct from that which line breaks are 'real'.

This seems to work, but, yeah, I know, weird:

diff --git a/_grab_ui.py b/_grab_ui.py
index f58f56a..284a30b 100644
--- a/_grab_ui.py
+++ b/_grab_ui.py
@@ -385,11 +385,12 @@ RegionTypeStr = str

 class GrabHandler(Handler):
     def __init__(self, args: Namespace, opts: Options,
-                 lines: List[str]) -> None:
+                 lines: List[str], line_breaks: List[bool]) -> None:
         super().__init__()
         self.args = args
         self.opts = opts
         self.lines = lines
+        self.line_breaks = line_breaks
         self.point = Position(args.x, args.y, args.top_line)
         self.mark = None           # type: Optional[Position]
         self.mark_type = NoRegion  # type: Type[Region]
@@ -617,8 +618,8 @@ class GrabHandler(Handler):

     def confirm(self, *args: Any) -> None:
         start, end = self._start_end()
-        self.result = {'copy': '\n'.join(
-            line_slice
+        self.result = {'copy': ''.join(
+            (line_slice + "\n" if self.line_breaks[line-1] else line_slice)
             for line in range(start.line, end.line + 1)
             for plain in [unstyled(self.lines[line - 1])]
             for start_x, end_x in [self.mark_type.selection_in_line(
@@ -655,11 +656,21 @@ type=int
     try:
         args, _rest = parse_args(args[1:], ospec)
         tty = open(os.ctermid())
-        lines = (sys.stdin.buffer.read().decode('utf-8')
+        lines_tmp = (sys.stdin.buffer.read().decode('utf-8')
                  .split('\n')[:-1])  # last line ends with \n, too
+        lines = []
+        line_breaks = []
+        for line in lines_tmp:
+            if line[-1:] == "\r":
+                lines.append(line[:-1])
+                line_breaks.append(False)
+            else:
+                lines.append(line)
+                line_breaks.append(True)
+
         sys.stdin = tty
         opts = load_config()
-        handler = GrabHandler(args, opts, lines)
+        handler = GrabHandler(args, opts, lines, line_breaks)
         loop = Loop()
         loop.loop(handler)
         return handler.result
diff --git a/grab.py b/grab.py
index 5ca77c5..935751a 100644
--- a/grab.py
+++ b/grab.py
@@ -19,15 +19,41 @@ def handle_result(args: List[str], data: Dict[str, Any], target_window_id: int,
     tab = window.tabref()
     if tab is None:
         return
-    content = window.as_text(as_ansi=True, add_history=True,
+
+    content_broken = window.as_text(as_ansi=True, add_history=True,
                              add_wrap_markers=True)
-    content = content.replace('\r\n', '\n').replace('\r', '\n')
-    n_lines = content.count('\n')
+    content_unbroken = window.as_text(as_ansi=True, add_history=True,
+                             add_wrap_markers=False)
+    content_broken = content_broken.replace('\r\n', '\n').replace('\r', '\n')
+    content_unbroken = content_unbroken.replace('\r\n', '\n').replace('\r', '\n')
+    content_broken = content_broken.split("\n")
+    content_unbroken = content_unbroken.split("\n")
+    broken_line_count = len(content_broken)
+    unbroken_line_count = len(content_unbroken)
+
+    i = 0
+    j = 0
+    content = []
+    while i < unbroken_line_count:
+        k = 0
+        lu = len(content_unbroken[i])
+        while True:
+            lb = len(content_broken[j])
+            if lb + k >= lu:
+                break
+            content.append(content_broken[j]+"\r\n")
+            k += lb
+            j += 1
+        content.append(content_broken[j]+"\n")
+        i += 1
+        j += 1
+
+    n_lines = len(content)
     top_line = (n_lines - (window.screen.lines - 1) - window.screen.scrolled_by)
     boss._run_kitten(_grab_ui.__file__, args=[
         '--title={}'.format(window.title),
         '--cursor-x={}'.format(window.screen.cursor.x),
         '--cursor-y={}'.format(window.screen.cursor.y),
         '--top-line={}'.format(top_line)],
-        input_data=content.encode('utf-8'),
+        input_data="".join(content).encode('utf-8'),
         window=window)
yurikhan commented 2 years ago

Oof. Excuse me, I need to test that over the weekend and maybe read some Kitty code to see if a better way is possible. But the concept seems sane enough.

yurikhan commented 2 years ago

Don't know why one would want to do a multi-line selection in mc.

Because why not. For example I could want to copy the whole mc screen into an HTML page, to make a non-graphical screenshot.

(It’s a two-sided issue though. Sometimes I have a wrapped file in mcview and what I really need is to have continuous lines, but Kitty won’t know they are wrapped by mcview. Other times, I have file with short lines but mcview pads them with spaces so I have to clean up trailing spaces.)