Skip to content

Navigation Menu

Sign in
Sign up

Fix multi_string_property: None when absent, snapshot lists (refids, REQUEST-STATUS) — part of #1722 - #1765

Draft
nihirborkar17 wants to merge 5 commits into
collective:main from
nihirborkar17:fix-multi-string-property-1722
Draft

Fix multi_string_property: None when absent, snapshot lists (refids, REQUEST-STATUS) — part of #1722 #1765
nihirborkar17 wants to merge 5 commits into
collective:main from
nihirborkar17:fix-multi-string-property-1722

Conversation

@nihirborkar17

@nihirborkar17 nihirborkar17 commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Linked issue

Description

This is the first PR in a planned series fixing the inconsistent
list-mutation behavior described in #1722. Full root-cause discussion
and the remaining checklist are in the issue thread.

Fixes multi_string_property (used by refids and REQUEST-STATUS):

  • fget returns None when the property is absent (previously [])
  • fget always returns a disconnected copy — mutating the returned
    list no longer affects the component. Previously this only worked
    when a single value was stored; with 2+ values, the getter returned
    a live reference into the component's internal storage, so
    .append() on the returned list silently mutated the component.
  • fset normalizes storage to always be list-shaped internally, even
    for a single value, closing off the root cause (Component.add()
    stores a bare scalar on first add, only becoming a list on the
    second add)
  • List elements are now consistently returned as plain str (not a
    mix of str/vText depending on how they were added)
  • Setting None or [] now deletes the property

Remaining work, tracked as follow-up PRs against #1722: RESOURCES
property (see also #1696), categories, attendees, rrules,
related_to, links, concepts, and CATEGORIES multi-group/LANGUAGE
support.

Checklist

  • I added a change log entry, following the instructions in all subsections under Change log requirements.
  • I followed icalendar's Artificial intelligence policy and disclosed my Responsible AI use in my commit messages, if applicable.
  • I added or updated tests, if applicable.
  • I ran and ensured all tests pass locally by following Run tests.
  • I added or edited documentation as necessary, both as docstrings to be rendered in the API documentation and narrative documentation, following the Style guide.

Additional information

See #1696 (open, adds RESOURCES via the same multi_string_property)
— that PR will inherit this fix regardless of merge order.

...ve#1722) AI: Claude Sonnet 5 used to explain the existing code and review my changes; code and tests written by me.
...onnet 5 used to explain the existing code and review my changes; code and tests written by me.
@github-actions github-actions Bot added the ai-suspicion This contribution is possibly created with lots of AI help without enough human understanding. label Sep 5, 2026

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

This pull request did not pass quality checks and AI use is suspected. Please review Contribute and make any necessary amendments.

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Profile summary:

GitHub user: nihirborkar17
🟢 No concerns found with user's profile.
🟢 No concerns found with recent PR activity.
🟢 No concerns found with recent issue activity.
For a more detailed report, run `gh-profiler nihirborkar17`.
Full profile
GitHub user: nihirborkar17
🟢 No concerns found with user's profile.
 🟢 Account age: 2 years
 🟢 Profile information:
 name: Nihir Borkar
 location: india
 bio: Exploring AI/ML and learning in public through open-source contributions.
 linkedin: https://www.linkedin.com/in/nihir-borkar/
 Empty fields: company, blog, email
🟢 No concerns found with recent PR activity.
 2 PRs opened in the last 21 days.
 0 opened against repos the user owns.
 0 opened against repos in publicly associated orgs.
 2 opened against external repos.
 🟢 1 of 2 external PRs merged in the last 21 days.
 🟢 0 of 2 external PRs closed without merging in the last 21 days.
🟢 No concerns found with recent issue activity.
 🟢 No new issues opened in the last 21 days.

read-the-docs-community Bot commented Sep 5, 2026
edited
Loading

Copy link
Copy Markdown

Copy link
Copy Markdown

Coverage Status

coverage: 97.569% (+0.007%) from 97.562% — nihirborkar17:fix-multi-string-property-1722 into collective:main

stevepiercy marked this pull request as draft September 6, 2026 12:25

Copy link
Copy Markdown
Member

I'm marking this as WIP until the actual desired direction is determined in the original issue #1722. This PR is premature. Once there is a decision made and a clear direction offered in #1722, then update this PR accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@niccokunzmann niccokunzmann Awaiting requested review from niccokunzmann niccokunzmann is a code owner
@stevepiercy stevepiercy Awaiting requested review from stevepiercy stevepiercy is a code owner
@angatha angatha Awaiting requested review from angatha angatha is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

ai-suspicion This contribution is possibly created with lots of AI help without enough human understanding.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /