From 7d74d9d9301f317b81ba1e81a3890b6f05e1cf85 Mon Sep 17 00:00:00 2001 From: Shane McDonald Date: Mon, 13 Oct 2025 09:05:13 -0400 Subject: [PATCH 1/3] Include attachments in TSV and JSON formatted output --- gcalcli/details.py | 51 +++++++++++- tests/test_details.py | 185 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 234 insertions(+), 2 deletions(-) create mode 100644 tests/test_details.py diff --git a/gcalcli/details.py b/gcalcli/details.py index acc4bac9..bdc5cb73 100644 --- a/gcalcli/details.py +++ b/gcalcli/details.py @@ -366,6 +366,52 @@ def _get(cls, event): return ACTION_DEFAULT +class Attachments(Handler): + """Handler for event attachments.""" + + fieldnames = ['attachments'] + + ATTACHMENT_PROPS = OrderedDict([('attachment_title', 'title'), + ('attachment_url', 'fileUrl')]) + + @classmethod + def fieldnames_data(cls): + return list(cls.ATTACHMENT_PROPS.keys()) + + @classmethod + def get(cls, event): + if 'attachments' not in event: + return [''] + + # For TSV output, use form feed (\f) to separate multiple attachments + # and pipe (|) to separate title from URL within each attachment. + # This follows the pattern suggested in issue #829 for handling + # list fields in TSV format. + # Format: "title1|url1\ftitle2|url2" + attachment_strs = [] + for attachment in event['attachments']: + title = attachment.get('title', '').strip() + url = attachment.get('fileUrl', '').strip() + # Escape any existing form feeds in the data + title = title.replace('\f', r'\f') + url = url.replace('\f', r'\f') + attachment_strs.append(f"{title}|{url}") + + return ['\f'.join(attachment_strs)] + + @classmethod + def data(cls, event): + attachments = event.get('attachments', []) + return [dict(zip(cls.ATTACHMENT_PROPS.keys(), + [attachment.get(prop, '') for prop in cls.ATTACHMENT_PROPS.values()])) + for attachment in attachments] + + @classmethod + def patch(cls, cal, event, fieldname, value): + # Attachments are read-only for now + raise ReadonlyCheckError(fieldname, cls.get(event), value) + + HANDLERS = OrderedDict([('id', ID), ('time', Time), ('length', Length), @@ -377,8 +423,9 @@ def _get(cls, event): ('calendar', Calendar), ('email', Email), ('attendees', Attendees), + ('attachments', Attachments), ('action', Action)]) -HANDLERS_READONLY = {Url, Calendar} +HANDLERS_READONLY = {Url, Calendar, Attachments} FIELD_HANDLERS = dict(chain.from_iterable( (((fieldname, handler) @@ -390,7 +437,7 @@ def _get(cls, event): in FIELD_HANDLERS.items() if handler in HANDLERS_READONLY) -_DETAILS_WITHOUT_HANDLERS = ['reminders', 'attachments', 'end'] +_DETAILS_WITHOUT_HANDLERS = ['reminders', 'end'] DETAILS = list(HANDLERS.keys()) + _DETAILS_WITHOUT_HANDLERS DETAILS_DEFAULT = {'time', 'title'} diff --git a/tests/test_details.py b/tests/test_details.py new file mode 100644 index 00000000..9290046e --- /dev/null +++ b/tests/test_details.py @@ -0,0 +1,185 @@ +"""Tests for detail handlers.""" + +import pytest +from gcalcli.details import ( + Attachments, + Attendees, + Title, + Conference, +) + + +class TestAttachmentsHandler: + """Tests for Attachments handler.""" + + def test_get_with_no_attachments(self): + """Test get() returns empty string for events without attachments.""" + event = {} + result = Attachments.get(event) + assert result == [''] + + def test_get_with_single_attachment(self): + """Test get() returns properly formatted string for single attachment.""" + event = { + 'attachments': [ + { + 'title': 'Notes by Gemini', + 'fileUrl': 'https://docs.google.com/document/d/123/edit' + } + ] + } + result = Attachments.get(event) + assert result == ['Notes by Gemini|https://docs.google.com/document/d/123/edit'] + + def test_get_with_multiple_attachments(self): + """Test get() returns properly formatted string for multiple attachments.""" + event = { + 'attachments': [ + { + 'title': 'Document 1', + 'fileUrl': 'https://docs.google.com/document/d/123/edit' + }, + { + 'title': 'Document 2', + 'fileUrl': 'https://docs.google.com/document/d/456/edit' + } + ] + } + result = Attachments.get(event) + expected = 'Document 1|https://docs.google.com/document/d/123/edit\fDocument 2|https://docs.google.com/document/d/456/edit' + assert result == [expected] + + def test_get_with_missing_fields(self): + """Test get() handles missing title/fileUrl fields gracefully.""" + event = { + 'attachments': [ + { + 'title': 'Document 1' + # missing fileUrl + }, + { + 'fileUrl': 'https://docs.google.com/document/d/456/edit' + # missing title + } + ] + } + result = Attachments.get(event) + expected = 'Document 1|\f|https://docs.google.com/document/d/456/edit' + assert result == [expected] + + def test_data_with_no_attachments(self): + """Test data() returns empty list for events without attachments.""" + event = {} + result = Attachments.data(event) + assert result == [] + + def test_data_with_single_attachment(self): + """Test data() returns properly formatted dict for single attachment.""" + event = { + 'attachments': [ + { + 'title': 'Notes by Gemini', + 'fileUrl': 'https://docs.google.com/document/d/123/edit' + } + ] + } + result = Attachments.data(event) + expected = [ + { + 'attachment_title': 'Notes by Gemini', + 'attachment_url': 'https://docs.google.com/document/d/123/edit' + } + ] + assert result == expected + + def test_data_with_multiple_attachments(self): + """Test data() returns properly formatted dict list for multiple attachments.""" + event = { + 'attachments': [ + { + 'title': 'Document 1', + 'fileUrl': 'https://docs.google.com/document/d/123/edit' + }, + { + 'title': 'Document 2', + 'fileUrl': 'https://docs.google.com/document/d/456/edit' + } + ] + } + result = Attachments.data(event) + expected = [ + { + 'attachment_title': 'Document 1', + 'attachment_url': 'https://docs.google.com/document/d/123/edit' + }, + { + 'attachment_title': 'Document 2', + 'attachment_url': 'https://docs.google.com/document/d/456/edit' + } + ] + assert result == expected + + def test_data_with_missing_fields(self): + """Test data() handles missing title/fileUrl fields gracefully.""" + event = { + 'attachments': [ + { + 'title': 'Document 1' + # missing fileUrl + }, + { + 'fileUrl': 'https://docs.google.com/document/d/456/edit' + # missing title + } + ] + } + result = Attachments.data(event) + expected = [ + { + 'attachment_title': 'Document 1', + 'attachment_url': '' + }, + { + 'attachment_title': '', + 'attachment_url': 'https://docs.google.com/document/d/456/edit' + } + ] + assert result == expected + + def test_get_with_semicolons(self): + """Test get() handles semicolons in titles and URLs correctly.""" + event = { + 'attachments': [ + { + 'title': 'Meeting Notes; Q4 2025', + 'fileUrl': 'https://example.com/doc;jsessionid=ABC123' + }, + { + 'title': 'Agenda', + 'fileUrl': 'https://docs.google.com/document/d/456/edit' + } + ] + } + result = Attachments.get(event) + # Semicolons should be preserved, form feed used as separator + expected = 'Meeting Notes; Q4 2025|https://example.com/doc;jsessionid=ABC123\fAgenda|https://docs.google.com/document/d/456/edit' + assert result == [expected] + + def test_get_with_form_feed_in_data(self): + """Test get() escapes existing form feed characters in data.""" + event = { + 'attachments': [ + { + 'title': 'Document\fwith\fformfeeds', + 'fileUrl': 'https://example.com/doc' + } + ] + } + result = Attachments.get(event) + # Form feeds in data should be escaped as r'\f' + expected = r'Document\fwith\fformfeeds|https://example.com/doc' + assert result == [expected] + + def test_fieldnames(self): + """Test fieldnames are correctly defined.""" + assert Attachments.fieldnames == ['attachments'] From 28e2f986bcaa1c7350c61a1b1c89d3771d659ec3 Mon Sep 17 00:00:00 2001 From: Shane McDonald Date: Tue, 28 Oct 2025 14:47:40 -0400 Subject: [PATCH 2/3] Fix linting issues in attachments code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove unused imports in tests/test_details.py - Fix line length violations in gcalcli/details.py and tests/test_details.py - Shorten docstrings and split long lines to meet 80 char limit 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- gcalcli/details.py | 11 ++++++++--- tests/test_details.py | 15 +++++++-------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/gcalcli/details.py b/gcalcli/details.py index bdc5cb73..01ad6572 100644 --- a/gcalcli/details.py +++ b/gcalcli/details.py @@ -402,9 +402,14 @@ def get(cls, event): @classmethod def data(cls, event): attachments = event.get('attachments', []) - return [dict(zip(cls.ATTACHMENT_PROPS.keys(), - [attachment.get(prop, '') for prop in cls.ATTACHMENT_PROPS.values()])) - for attachment in attachments] + return [ + dict(zip( + cls.ATTACHMENT_PROPS.keys(), + [attachment.get(prop, '') + for prop in cls.ATTACHMENT_PROPS.values()] + )) + for attachment in attachments + ] @classmethod def patch(cls, cal, event, fieldname, value): diff --git a/tests/test_details.py b/tests/test_details.py index 9290046e..e8e60b6f 100644 --- a/tests/test_details.py +++ b/tests/test_details.py @@ -1,11 +1,7 @@ """Tests for detail handlers.""" -import pytest from gcalcli.details import ( Attachments, - Attendees, - Title, - Conference, ) @@ -19,7 +15,7 @@ def test_get_with_no_attachments(self): assert result == [''] def test_get_with_single_attachment(self): - """Test get() returns properly formatted string for single attachment.""" + """Test get() returns formatted string for single attachment.""" event = { 'attachments': [ { @@ -32,7 +28,7 @@ def test_get_with_single_attachment(self): assert result == ['Notes by Gemini|https://docs.google.com/document/d/123/edit'] def test_get_with_multiple_attachments(self): - """Test get() returns properly formatted string for multiple attachments.""" + """Test get() returns formatted string for multiple attachments.""" event = { 'attachments': [ { @@ -46,7 +42,10 @@ def test_get_with_multiple_attachments(self): ] } result = Attachments.get(event) - expected = 'Document 1|https://docs.google.com/document/d/123/edit\fDocument 2|https://docs.google.com/document/d/456/edit' + expected = ( + 'Document 1|https://docs.google.com/document/d/123/edit\f' + 'Document 2|https://docs.google.com/document/d/456/edit' + ) assert result == [expected] def test_get_with_missing_fields(self): @@ -93,7 +92,7 @@ def test_data_with_single_attachment(self): assert result == expected def test_data_with_multiple_attachments(self): - """Test data() returns properly formatted dict list for multiple attachments.""" + """Test data() returns formatted dict list for attachments.""" event = { 'attachments': [ { From eac4069bc56621a627bd47a690795f259ddc981d Mon Sep 17 00:00:00 2001 From: Shane McDonald Date: Tue, 28 Oct 2025 14:50:26 -0400 Subject: [PATCH 3/3] Fix Attachments.patch() to only error on value changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous implementation unconditionally raised ReadonlyCheckError, which prevented updating any event with attachments, even when modifying other fields. This broke the workflow of exporting to TSV, modifying other fields, and importing back. Now follows the same pattern as other read-only handlers (like Url): only raise an error when the attachment value actually changes. Added tests to verify: - Patching with unchanged value doesn't raise error - Patching with changed value raises ReadonlyCheckError - Patching events without attachments works correctly Fixes review feedback from https://github.com/insanum/gcalcli/pull/847#discussion_r2458599062 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- gcalcli/details.py | 5 ++++- tests/test_details.py | 41 ++++++++++++++++++++++++++++++++++++++--- 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/gcalcli/details.py b/gcalcli/details.py index 01ad6572..47313ef2 100644 --- a/gcalcli/details.py +++ b/gcalcli/details.py @@ -414,7 +414,10 @@ def data(cls, event): @classmethod def patch(cls, cal, event, fieldname, value): # Attachments are read-only for now - raise ReadonlyCheckError(fieldname, cls.get(event), value) + current_value = cls.get(event)[0] + + if current_value != value: + raise ReadonlyCheckError(fieldname, current_value, value) HANDLERS = OrderedDict([('id', ID), diff --git a/tests/test_details.py b/tests/test_details.py index e8e60b6f..2823756a 100644 --- a/tests/test_details.py +++ b/tests/test_details.py @@ -1,8 +1,8 @@ """Tests for detail handlers.""" -from gcalcli.details import ( - Attachments, -) +import pytest +from gcalcli.details import Attachments +from gcalcli.exceptions import ReadonlyCheckError class TestAttachmentsHandler: @@ -182,3 +182,38 @@ def test_get_with_form_feed_in_data(self): def test_fieldnames(self): """Test fieldnames are correctly defined.""" assert Attachments.fieldnames == ['attachments'] + + def test_patch_with_unchanged_value(self): + """Test patch doesn't error when value hasn't changed.""" + event = { + 'attachments': [ + { + 'title': 'Notes by Gemini', + 'fileUrl': 'https://docs.google.com/document/d/123/edit' + } + ] + } + current_value = 'Notes by Gemini|https://docs.google.com/document/d/123/edit' + # Should not raise an error when value hasn't changed + Attachments.patch(None, event, 'attachments', current_value) + + def test_patch_with_changed_value(self): + """Test patch raises error when trying to change value.""" + event = { + 'attachments': [ + { + 'title': 'Notes by Gemini', + 'fileUrl': 'https://docs.google.com/document/d/123/edit' + } + ] + } + new_value = 'Different|https://example.com' + # Should raise ReadonlyCheckError when trying to change value + with pytest.raises(ReadonlyCheckError): + Attachments.patch(None, event, 'attachments', new_value) + + def test_patch_with_no_attachments(self): + """Test patch handles events without attachments.""" + event = {} + # Should not raise an error for empty value + Attachments.patch(None, event, 'attachments', '')