More work on dnd kitten

kittens/dnd/drop.go

@@ -146,19 +146,19 @@ func do_local_copy(ctx context.Context, dest_dir *os.File, uri_list []string) (e
 		if src_file != nil {
 			src_file.Close()
 		}
-		if src_file, err = os.Open(path); err != nil {
-			return err
-		}
-		st, err := src_file.Stat()
+		st, err := os.Lstat(path)
 		if err != nil {
 			return err
 		}
 		if st.IsDir() {
-			d, err := utils.CreateDirAt(dest_dir, filepath.Base(src_file.Name()), st.Mode().Perm())
+			if src_file, err = os.Open(path); err != nil {
+				return err
+			}
+			d, err := utils.CreateDirAt(dest_dir, filepath.Base(path), st.Mode().Perm())
 			if err != nil {
 				return err
 			}
-			err = utils.CopyFolderContents(ctx, src_file, dest_dir, utils.CopyFolderOptions{
+			err = utils.CopyFolderContents(ctx, src_file, d, utils.CopyFolderOptions{
 				Filter_files: func(parent *os.File, child os.FileInfo) bool {
 					return child.IsDir() || child.Mode().IsRegular() || child.Mode()&fs.ModeSymlink != 0
 				},
@@ -169,10 +169,14 @@ func do_local_copy(ctx context.Context, dest_dir *os.File, uri_list []string) (e
 			}
 		} else if st.Mode().IsRegular() {
 			// First try a hard link
-			if err = os.Link(src_file.Name(), filepath.Join(dest_dir.Name(), filepath.Base(src_file.Name()))); err == nil {
+			dest := filepath.Join(dest_dir.Name(), filepath.Base(path))
+			if err = os.Link(path, dest); err == nil {
 				continue
 			}
-			d, err := utils.CreateAt(dest_dir, filepath.Base(src_file.Name()), st.Mode().Perm())
+			if src_file, err = os.Open(path); err != nil {
+				return err
+			}
+			d, err := utils.CreateAt(dest_dir, filepath.Base(dest), st.Mode().Perm())
 			if err != nil {
 				return err
 			}
@@ -181,6 +185,15 @@ func do_local_copy(ctx context.Context, dest_dir *os.File, uri_list []string) (e
 			if err != nil {
 				return err
 			}
+		} else if st.Mode()&fs.ModeSymlink != 0 {
+			target, err := os.Readlink(path)
+			if err != nil {
+				return err
+			}
+			dest := filepath.Join(dest_dir.Name(), filepath.Base(path))
+			if err := os.Symlink(target, dest); err != nil {
+				return err
+			}
 		}
 	}
 	return
@@ -280,7 +293,6 @@ type drop_status struct {
 
 	local_copy struct {
 		ctx        context.Context
-		dest_dir   *os.File
 		cancel_ctx context.CancelFunc
 		completion chan error
 	}
@@ -347,6 +359,24 @@ func (dnd *dnd) all_drop_data_received() {
 	dnd.end_drop()
 }
 
+func (dnd *dnd) drop_on_wakeup() error {
+	if dnd.drop_status.local_copy.completion == nil {
+		return nil
+	}
+	select {
+	case err := <-dnd.drop_status.local_copy.completion:
+		dnd.drop_status.local_copy.ctx = nil
+		dnd.drop_status.local_copy.completion = nil
+		if err != nil {
+			return err
+		}
+		dnd.all_drop_data_received()
+		return nil
+	default:
+		return nil
+	}
+}
+
 func (dnd *dnd) new_tdir() (dir_file *os.File, err error) {
 	dnd.tdir_counter++
 	name := strconv.Itoa(dnd.tdir_counter)
@@ -355,12 +385,6 @@ func (dnd *dnd) new_tdir() (dir_file *os.File, err error) {
 
 func (dnd *dnd) all_mime_data_dropped() (err error) {
 	drop_status := &dnd.drop_status
-	if s, found := dnd.drop_dests["text/uri-list"]; found {
-		b := s.dest.(*bufferWriteCloser)
-		if drop_status.uri_list, err = parse_uri_list(b.String()); err != nil {
-			return err
-		}
-	}
 	if len(drop_status.uri_list) == 0 {
 		dnd.data_has_been_dropped = true
 		dnd.end_drop()
@@ -396,7 +420,6 @@ func (dnd *dnd) all_mime_data_dropped() (err error) {
 		}
 		drop_status.open_remote_dir = drop_status.root_remote_dir
 	} else {
-		drop_status.local_copy.dest_dir = f
 		drop_status.local_copy.ctx, drop_status.local_copy.cancel_ctx = context.WithCancel(context.Background())
 		drop_status.local_copy.completion = make(chan error, 1)
 		go do_local_copy_in_goroutine(drop_status.local_copy.ctx, f, drop_status.local_copy.completion, slices.Clone(drop_status.uri_list), func() { dnd.lp.WakeupMainThread() })
@@ -569,6 +592,13 @@ func (dnd *dnd) on_drop_data(cmd DC) error {
 			return err
 		}
 		dest.completed = true
+		if mime == "text/uri-list" {
+			b := dest.dest.(*bufferWriteCloser)
+			var err error
+			if drop_status.uri_list, err = parse_uri_list(b.String()); err != nil {
+				return err
+			}
+		}
 		pending := false
 		for _, d := range dnd.drop_dests {
 			if !d.completed {

kittens/dnd/main.go

@@ -168,6 +168,13 @@ func (dnd *dnd) run_loop() (err error) {
 		return ""
 	}
 
+	dnd.lp.OnWakeup = func() error {
+		if err := dnd.drop_on_wakeup(); err != nil {
+			return err
+		}
+		return nil
+	}
+
 	dnd.lp.OnDnDData = func(cmd loop.DndCommand) error {
 		// TODO: Use lp.QueueDnDData to implement drag and drop protocol
 		// If allow_drags, start a drag when the terminal sends the t=o
@@ -176,35 +183,12 @@ func (dnd *dnd) run_loop() (err error) {
 		// reset drag_started at the end of the drag. Use opts.DragAction to
 		// set what actions are allowed.
 
-		// If a drop enters the window and has one or more MIME types present
-		// in drop_dests, accept the drop, unless drag_started is true.
-
-		// Redraw the screen whenever drag or drop status changes.
-
-		// When a drop happens, write all data for the MIME types present in
-		// both drop_dests and the actual dropped data. For the text/uri-list
-		// type if the terminal indicates it is coming from a remote machine
-		// request the data for the file:// entries from the uri-list using the
-		// dnd protocol and write it, otherwise, copy the file URLs using
-		// normal file system operations. If opts.ConfirmDropOverwrite is true
-		// then when some data would overwrite existing file, put it into a
-		// temp file instead and after all data is transferred as the user for
-		// confirmation and overwrite or not accordingly. While a drop is in
-		// progress the render_screen() function should hide the drop
-		// destination buttons and instead show the text "Drop in progress,
-		// reading data..."
-		// Be very careful when writing dropped data from uri-list nothing
-		// should be written outside the destination directory (the current
-		// working directory by default). In particular, symlinks must be
-		// handled with care.
-
 		// When acting as a drag source, dont forget to implement support for
 		// remote dragging, which means providing data for the text/uri-list
 		// mime type file:// entries when the terminal requests it using the
 		// dnd protocol. If the action chosen is move, delete the files
 		// corresponding to the drag sources, including the files in the
 		// uri-list and exit.
-
 		switch cmd.Type {
 		case 'T':
 			switch string(cmd.Payload) {

kitty_tests/__init__.py

@@ -408,6 +408,15 @@ class PTY:
                 os.close(self.slave_fd)
                 del self.slave_fd
             if self.child_pid > 0 and not self.child_waited_for:
+                st = time.monotonic()
+                while time.monotonic() - st < 2:
+                    pid, ec = os.waitpid(self.child_pid, os.WNOHANG)
+                    if pid == self.child_pid:
+                        self.child_waited_for = True
+                        break
+                    time.sleep(0.1)
+                if not self.child_waited_for:
+                    os.kill(self.child_pid, signal.SIGKILL)
                     os.waitpid(self.child_pid, 0)
                     self.child_waited_for = True
 
@@ -453,7 +462,9 @@ class PTY:
             msg = 'The condition was not met'
             if timeout_msg is not None:
                 msg = timeout_msg()
-            raise TimeoutError(f'Timed out after {timeout} seconds: {msg}. {self.screen_contents_for_error()}')
+            if not msg.endswith('\n'):
+                msg += '. '
+            raise TimeoutError(f'Timed out after {timeout} seconds: {msg}{self.screen_contents_for_error()}')
 
     def wait_till_child_exits(self, timeout=30 if BaseTest.is_ci else 10, require_exit_code=None):
         end_time = time.monotonic() + timeout

kitty_tests/dnd_kitten.py

@@ -129,6 +129,8 @@ class TestDnDKitten(BaseTest):
 
     def reset_kitten(self, remote_client: bool, clear_tdir=True):
         if clear_tdir:
+            self.send_dnd_command_to_kitten('PING')
+            self.wait_for_responses('PONG')
             shutil.rmtree(self.kitten_wd)
             os.mkdir(self.kitten_wd)
             shutil.rmtree(self.src_data_dir)
@@ -160,7 +162,7 @@ class TestDnDKitten(BaseTest):
         def wait_till():
             return q == self.messages_from_kitten.strip()
         try:
-            self.pty.wait_till(wait_till, timeout, lambda: f'Responses so far: Expected:\n{q!r}\nActual:\n{self.messages_from_kitten.strip()!r} != {q!r}')
+            self.pty.wait_till(wait_till, timeout, lambda: f'Responses so far: Expected:\n{q!r}\nActual:\n{self.messages_from_kitten.strip()!r}\n')
         finally:
             self.messages_from_kitten = ''
 
@@ -249,16 +251,21 @@ class TestDnDKitten(BaseTest):
                 dnd_test_fake_drop_data(self.capture.window_id, mime, chunk, 0, True)
             dnd_test_fake_drop_data(self.capture.window_id, mime, b'')
 
+        self.send_dnd_command_to_kitten('DROP_IS_REMOTE')
+        self.wait_for_responses(str(remote_client))
+
         with open(os.path.join(self.src_data_dir, 'some-image.png'), 'rb') as f:
             send_file_in_chunks(f, 'image/png', 1117)
 
-        self.send_dnd_command_to_kitten('DROP_IS_REMOTE')
-        self.wait_for_responses(str(remote_client))
         self.send_dnd_command_to_kitten('DROP_URI_LIST')
         self.wait_for_responses('|'.join(path_list))
         jn = os.path.join
         self.assert_files_have_same_content(jn(self.src_data_dir, 'some-image.png'), jn(self.kitten_wd, img_drop_path))
         shutil.rmtree(os.path.dirname(jn(self.kitten_wd, img_drop_path)))
+        if remote_client:
+            pass
+        else:
+            self.wait_for_state('drop_action', 0)
 
     def assert_files_have_same_content(self, a, b):
         with open(a, 'rb') as fa, open(b, 'rb') as fb: