mirrors/kitty
1
0
Fork 0
mirror of https://github.com/kovidgoyal/kitty synced 2026-06-08 14:18:26 +02:00
Code Issues Packages Projects Releases Wiki Activity

Revert to 1-based directory entry indexing in DnD protocol

Browse Source 
- dnd.c: entry_num==0 closes handle; entry_num>=1 reads at entry_num-1
- parse-dnd-command.h: revert cell_y default to 0 (from {0} init)
- gen/apc_parsers.py: remove post_init for DnD parser
- docs/dnd-protocol.rst: update to say 1-based indexing
- tests: all .index() calls add +1, rename test_dir_entry_one_based_index

Agent-Logs-Url: https://github.com/kovidgoyal/kitty/sessions/d4074aba-3aeb-4d2b-adc1-d6c6f2b539e7

Co-authored-by: kovidgoyal <1308621+kovidgoyal@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot]
2026-04-09 11:52:31 +00:00
committed by GitHub
parent 7e0e3eb6ac
commit 4d2b63fce8
5 changed files with 29 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
docs/dnd-protocol.rst
View File

@@ -179,8 +179,8 @@ encoded and might be chunked if the directory has a lot of entries.
``idx`` is an arbitrary 32 bit integer that acts as a handle to this ``idx`` is an arbitrary 32 bit integer that acts as a handle to this
directory. The client can now read the files in this directory using requests of the form directory. The client can now read the files in this directory using requests of the form
``t=d:x=idx:y=num:r=request_id``, here ``num`` is the 0-based index into the list of ``t=d:x=idx:y=num:r=request_id``, here ``num`` is the 1-based index into the list of
directory entries previously transmitted to the client, where, ``0`` will directory entries previously transmitted to the client, where, ``1`` will
correspond to the first entry in the directory. Once the client is done correspond to the first entry in the directory. Once the client is done
reading a directory it should transmit ``t=d:x=idx:r=request_id`` to the terminal. The reading a directory it should transmit ``t=d:x=idx:r=request_id`` to the terminal. The
terminal can then free any resources associated with that directory. The terminal can then free any resources associated with that directory. The

2
gen/apc_parsers.py
View File

@@ -345,7 +345,7 @@ def parsers() -> None:
} }
text = generate( text = generate(
'parse_dnd_code', 'screen_handle_dnd_command', 'dnd_command', keymap, 'DnDCommand', 'parse_dnd_code', 'screen_handle_dnd_command', 'dnd_command', keymap, 'DnDCommand',
payload_is_base64=False, start_parsing_at=0, field_sep=':', post_init='g.cell_y = -1;') payload_is_base64=False, start_parsing_at=0, field_sep=':')
write_header(text, 'kitty/parse-dnd-command.h') write_header(text, 'kitty/parse-dnd-command.h')

8
kitty/dnd.c
View File

@@ -872,7 +872,7 @@ drop_request_uri_data(Window *w, const char *payload, size_t payload_sz) {
/* Handle a t=d request from the client. /* Handle a t=d request from the client.
* handle_id: the directory handle (x= key). * handle_id: the directory handle (x= key).
* entry_num: <0 means close the handle; >=0 means read that entry (0-based). * entry_num: 0 means close the handle; >=1 means read that entry (1-based).
* Returns true if completed synchronously, false if async file I/O started. */ * Returns true if completed synchronously, false if async file I/O started. */
static bool static bool
do_drop_handle_dir_request(Window *w, uint32_t handle_id, int32_t entry_num) { do_drop_handle_dir_request(Window *w, uint32_t handle_id, int32_t entry_num) {
@@ -881,7 +881,7 @@ do_drop_handle_dir_request(Window *w, uint32_t handle_id, int32_t entry_num) {
DirHandle *h = drop_find_dir_handle(w, handle_id); DirHandle *h = drop_find_dir_handle(w, handle_id);
if (!h) { drop_send_error(w, EINVAL); return true; } if (!h) { drop_send_error(w, EINVAL); return true; }
if (entry_num < 0) { if (entry_num == 0) {
/* Close the handle */ /* Close the handle */
size_t hidx = (size_t)(h - w->drop.dir_handles); size_t hidx = (size_t)(h - w->drop.dir_handles);
drop_free_dir_handle(h); drop_free_dir_handle(h);
@@ -889,8 +889,8 @@ do_drop_handle_dir_request(Window *w, uint32_t handle_id, int32_t entry_num) {
return true; return true;
} }
/* Read the entry at 0-based index */ /* Read the entry at 1-based index */
size_t eidx = (size_t)entry_num; size_t eidx = (size_t)(entry_num - 1);
if (eidx >= h->num_entries) { drop_send_error(w, ENOENT); return true; } if (eidx >= h->num_entries) { drop_send_error(w, ENOENT); return true; }
char full[PATH_MAX]; char full[PATH_MAX];

1
kitty/parse-dnd-command.h
View File

@@ -11,7 +11,6 @@ static inline void parse_dnd_code(PS *self, uint8_t *parser_buf,
enum PARSER_STATES { KEY, EQUAL, UINT, INT, FLAG, AFTER_VALUE, PAYLOAD }; enum PARSER_STATES { KEY, EQUAL, UINT, INT, FLAG, AFTER_VALUE, PAYLOAD };
enum PARSER_STATES state = KEY, value_state = FLAG; enum PARSER_STATES state = KEY, value_state = FLAG;
DnDCommand g = {0}; DnDCommand g = {0};
g.cell_y = -1;
unsigned int i, code; unsigned int i, code;
uint64_t lcode; uint64_t lcode;
int64_t accumulator; int64_t accumulator;

44
kitty_tests/dnd.py
View File

@@ -74,7 +74,7 @@ def client_dir_read(handle_id: int, entry_num: int | None = None, client_id: int
"""Escape code for a directory request (t=d:x=handle_id[:y=entry_num]). """Escape code for a directory request (t=d:x=handle_id[:y=entry_num]).
* entry_num=None → close the directory handle. * entry_num=None → close the directory handle.
* entry_num>=0 → read that entry (0-based). * entry_num>=1 → read that entry (1-based).
""" """
meta = f'{DND_CODE};t=d:x={handle_id}' meta = f'{DND_CODE};t=d:x={handle_id}'
if entry_num is not None: if entry_num is not None:
@@ -779,10 +779,10 @@ class TestDnDProtocol(BaseTest):
self.assertIn('a.txt', entry_names) self.assertIn('a.txt', entry_names)
self.assertIn('b', entry_names) self.assertIn('b', entry_names)
# Find index of 'a.txt' in the entries list (0-based for t=d:y=) # Find index of 'a.txt' in the entries list (1-based for t=d:y=)
entries_list = [e.decode() for e in root_entries] entries_list = [e.decode() for e in root_entries]
a_idx = entries_list.index('a.txt') a_idx = entries_list.index('a.txt') + 1
b_idx = entries_list.index('b') b_idx = entries_list.index('b') + 1
# Read a.txt # Read a.txt
parse_bytes(screen, client_dir_read(root_handle_id, a_idx)) parse_bytes(screen, client_dir_read(root_handle_id, a_idx))
@@ -811,8 +811,8 @@ class TestDnDProtocol(BaseTest):
self.assertIn('d', b_names) self.assertIn('d', b_names)
b_entries_list = [e.decode() for e in b_entries] b_entries_list = [e.decode() for e in b_entries]
bc_idx = b_entries_list.index('c.txt') bc_idx = b_entries_list.index('c.txt') + 1
bd_idx = b_entries_list.index('d') bd_idx = b_entries_list.index('d') + 1
# Read b/c.txt (binary integrity) # Read b/c.txt (binary integrity)
parse_bytes(screen, client_dir_read(b_handle_id, bc_idx)) parse_bytes(screen, client_dir_read(b_handle_id, bc_idx))
@@ -841,7 +841,7 @@ class TestDnDProtocol(BaseTest):
self.assertIn('e.txt', bd_names) self.assertIn('e.txt', bd_names)
bd_entries_list = [e.decode() for e in bd_entries] bd_entries_list = [e.decode() for e in bd_entries]
bde_idx = bd_entries_list.index('e.txt') bde_idx = bd_entries_list.index('e.txt') + 1
# Read b/d/e.txt # Read b/d/e.txt
parse_bytes(screen, client_dir_read(bd_handle_id, bde_idx)) parse_bytes(screen, client_dir_read(bd_handle_id, bde_idx))
@@ -879,7 +879,7 @@ class TestDnDProtocol(BaseTest):
self._assert_no_output(cap, wid) self._assert_no_output(cap, wid)
# Now try to read from the closed handle → EINVAL # Now try to read from the closed handle → EINVAL
parse_bytes(screen, client_dir_read(hid, 0)) parse_bytes(screen, client_dir_read(hid, 1))
events = self._get_events(cap, wid) events = self._get_events(cap, wid)
self.assertEqual(len(events), 1) self.assertEqual(len(events), 1)
self.ae(events[0]['type'], 'R') self.ae(events[0]['type'], 'R')
@@ -950,7 +950,7 @@ class TestDnDProtocol(BaseTest):
entries = [e.decode() for e in payload.split(b'\x00') if e] entries = [e.decode() for e in payload.split(b'\x00') if e]
self.assertIn('link.txt', entries) self.assertIn('link.txt', entries)
self.assertIn('real.txt', entries) self.assertIn('real.txt', entries)
link_idx = entries.index('link.txt') link_idx = entries.index('link.txt') + 1
# Read the symlink entry → should get t=r with X=1 and target path # Read the symlink entry → should get t=r with X=1 and target path
parse_bytes(screen, client_dir_read(hid, link_idx)) parse_bytes(screen, client_dir_read(hid, link_idx))
@@ -985,7 +985,7 @@ class TestDnDProtocol(BaseTest):
hid = int(d_ev[0]['meta']['x']) hid = int(d_ev[0]['meta']['x'])
entries = [e.decode() for e in payload.split(b'\x00') if e] entries = [e.decode() for e in payload.split(b'\x00') if e]
self.assertIn('link_to_dir', entries) self.assertIn('link_to_dir', entries)
link_idx = entries.index('link_to_dir') link_idx = entries.index('link_to_dir') + 1
# Read the symlink → should get t=r with X=1 # Read the symlink → should get t=r with X=1
parse_bytes(screen, client_dir_read(hid, link_idx)) parse_bytes(screen, client_dir_read(hid, link_idx))
@@ -1018,7 +1018,7 @@ class TestDnDProtocol(BaseTest):
) )
hid = int(d_ev[0]['meta']['x']) hid = int(d_ev[0]['meta']['x'])
entries = [e.decode() for e in payload.split(b'\x00') if e] entries = [e.decode() for e in payload.split(b'\x00') if e]
link_idx = entries.index('abs_link.txt') link_idx = entries.index('abs_link.txt') + 1
parse_bytes(screen, client_dir_read(hid, link_idx)) parse_bytes(screen, client_dir_read(hid, link_idx))
raw = cap.consume(wid) raw = cap.consume(wid)
@@ -1048,7 +1048,7 @@ class TestDnDProtocol(BaseTest):
) )
hid = int(d_ev[0]['meta']['x']) hid = int(d_ev[0]['meta']['x'])
entries = [e.decode() for e in payload.split(b'\x00') if e] entries = [e.decode() for e in payload.split(b'\x00') if e]
reg_idx = entries.index('regular.txt') reg_idx = entries.index('regular.txt') + 1
parse_bytes(screen, client_dir_read(hid, reg_idx)) parse_bytes(screen, client_dir_read(hid, reg_idx))
raw = cap.consume(wid) raw = cap.consume(wid)
@@ -1083,7 +1083,7 @@ class TestDnDProtocol(BaseTest):
entries = [e.decode() for e in payload.split(b'\x00') if e] entries = [e.decode() for e in payload.split(b'\x00') if e]
# Read regular file # Read regular file
data_idx = entries.index('data.bin') data_idx = entries.index('data.bin') + 1
parse_bytes(screen, client_dir_read(hid, data_idx)) parse_bytes(screen, client_dir_read(hid, data_idx))
raw = cap.consume(wid) raw = cap.consume(wid)
events = parse_escape_codes_b64(raw) events = parse_escape_codes_b64(raw)
@@ -1093,7 +1093,7 @@ class TestDnDProtocol(BaseTest):
b'\x00\x01\x02\x03') b'\x00\x01\x02\x03')
# Read symlink # Read symlink
alias_idx = entries.index('alias.bin') alias_idx = entries.index('alias.bin') + 1
parse_bytes(screen, client_dir_read(hid, alias_idx)) parse_bytes(screen, client_dir_read(hid, alias_idx))
raw = cap.consume(wid) raw = cap.consume(wid)
events = parse_escape_codes_b64(raw) events = parse_escape_codes_b64(raw)
@@ -1124,7 +1124,7 @@ class TestDnDProtocol(BaseTest):
) )
root_hid = int(d_ev[0]['meta']['x']) root_hid = int(d_ev[0]['meta']['x'])
entries = [e.decode() for e in payload.split(b'\x00') if e] entries = [e.decode() for e in payload.split(b'\x00') if e]
sub_idx = entries.index('sub') sub_idx = entries.index('sub') + 1
# Open subdirectory # Open subdirectory
parse_bytes(screen, client_dir_read(root_hid, sub_idx)) parse_bytes(screen, client_dir_read(root_hid, sub_idx))
@@ -1138,7 +1138,7 @@ class TestDnDProtocol(BaseTest):
sub_entries = [e.decode() for e in sub_payload.split(b'\x00') if e] sub_entries = [e.decode() for e in sub_payload.split(b'\x00') if e]
self.assertIn('nested_link.txt', sub_entries) self.assertIn('nested_link.txt', sub_entries)
link_idx = sub_entries.index('nested_link.txt') link_idx = sub_entries.index('nested_link.txt') + 1
parse_bytes(screen, client_dir_read(sub_hid, link_idx)) parse_bytes(screen, client_dir_read(sub_hid, link_idx))
raw = cap.consume(wid) raw = cap.consume(wid)
events = parse_escape_codes_b64(raw) events = parse_escape_codes_b64(raw)
@@ -1147,8 +1147,8 @@ class TestDnDProtocol(BaseTest):
self.ae(b''.join(e['payload'] for e in r_events if e['payload']), self.ae(b''.join(e['payload'] for e in r_events if e['payload']),
b'target.txt') b'target.txt')
def test_dir_entry_zero_based_index(self) -> None: def test_dir_entry_one_based_index(self) -> None:
"""Directory entry index 0 reads the first entry (0-based).""" """Directory entry index 1 reads the first entry (1-based)."""
import os import os
import tempfile import tempfile
with tempfile.TemporaryDirectory() as root: with tempfile.TemporaryDirectory() as root:
@@ -1163,12 +1163,12 @@ class TestDnDProtocol(BaseTest):
d_ev = [e for e in events if e['type'] == 'd'] d_ev = [e for e in events if e['type'] == 'd']
hid = int(d_ev[0]['meta']['x']) hid = int(d_ev[0]['meta']['x'])
# Index 0 should read the first entry # Index 1 should read the first entry
parse_bytes(screen, client_dir_read(hid, 0)) parse_bytes(screen, client_dir_read(hid, 1))
raw = cap.consume(wid) raw = cap.consume(wid)
events = parse_escape_codes_b64(raw) events = parse_escape_codes_b64(raw)
r_events = [e for e in events if e['type'] == 'r'] r_events = [e for e in events if e['type'] == 'r']
self.assertTrue(r_events, 'entry index 0 should read the first entry') self.assertTrue(r_events, 'entry index 1 should read the first entry')
data = b''.join(e['payload'] for e in r_events if e['payload']) data = b''.join(e['payload'] for e in r_events if e['payload'])
self.ae(data, b'first file') self.ae(data, b'first file')
@@ -1979,7 +1979,7 @@ class TestDnDProtocol(BaseTest):
handle_id = int(d_events[0]['meta']['x']) handle_id = int(d_events[0]['meta']['x'])
listing = b''.join(chunk for e in d_events for chunk in e['chunks'] if chunk) listing = b''.join(chunk for e in d_events for chunk in e['chunks'] if chunk)
entries = [e.decode() for e in listing.split(b'\x00') if e] entries = [e.decode() for e in listing.split(b'\x00') if e]
f_idx = entries.index('f.txt') f_idx = entries.index('f.txt') + 1
# Read file with request_id # Read file with request_id
parse_bytes(screen, client_dir_read(handle_id, f_idx, request_id=33)) parse_bytes(screen, client_dir_read(handle_id, f_idx, request_id=33))