You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
append() and extend_list() had identical try/except blocks: attempt the operation, catch PatchError, check for FLOW_SEQUENCE or NOT_A_SEQUENCE, fall back to get→mutate→replace. Extracted into _with_flow_seq_fallback(keys, patches, fallback_fn).
insert() is unchanged — it uses BLOCK_SEQUENCE_EXPECTED (a single kind covering both the flow and non-sequence cases) and needs an isinstance check, so it doesn't fit the same pattern.
Net: −22 lines, append and extend_list each collapse to two lines.
Closing this — on reflection the helper is too shallow to justify the indirection.
The function interface is clunky: callers still compute
oute for patch construction, then the helper recomputes it independently in the fallback path. insert() doesn't fit the pattern at all (different error kind + isinstance check), so the abstraction doesn't generalize. For only 2 call sites with slightly different lambdas, the inline duplication is actually easier to follow — the reader sees the full error-handling story in one place without jumping to a callback-based helper.
The original 9-12 line try/except blocks in �ppend and �xtend_list are straightforward and self-contained. The cost of the duplication is low; the cost of the indirection (lambda stack frames, split control flow, awkward parameter set) outweighs it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses
doc/todo/flow-sequence-fallback-duplication.md.append()andextend_list()had identical try/except blocks: attempt the operation, catchPatchError, check forFLOW_SEQUENCEorNOT_A_SEQUENCE, fall back to get→mutate→replace. Extracted into_with_flow_seq_fallback(keys, patches, fallback_fn).insert()is unchanged — it usesBLOCK_SEQUENCE_EXPECTED(a single kind covering both the flow and non-sequence cases) and needs anisinstancecheck, so it doesn't fit the same pattern.Net: −22 lines,
appendandextend_listeach collapse to two lines.