Consolidate duplicated style utility functions into shared module#1
Open
imakris wants to merge 7 commits into
Open
Consolidate duplicated style utility functions into shared module#1imakris wants to merge 7 commits into
imakris wants to merge 7 commits into
Conversation
The five largest tools (~7,200 LOC combined) carried 30+ byte-identical copies of string-aware scanners, paren matchers, signature predicates, and return-block helpers. Extract them into _style_utils, then have the tools import rather than redefine. Also collapse a handful of file-local clumsy patterns flagged by the audit: - table-drive the fix_hanging_indent transform pipeline rather than chaining 10+ "if replacement is not None" blocks - merge the duplicate lambda-body sibling walks into one parameterized pass - factor wrap_long_conditions's repeated keyword-detect + paren-finding preamble into find_control_header_close + apply_*_edits helpers - fold the four rendered_lines_fit_* variants in format_member_declarations into one with optional params - replace the three angle-aware scanners in format_member_declarations with a shared _scan_angle_aware helper Totals: 7,213 lines across the 5 files reduced to 5,280, with a new 575-line _style_utils. Net -1,358 lines, all 311 tests still pass. https://claude.ai/code/session_01JmfhcVugXuQsgxnqZfDjdi
Keeps pytest-generated __pycache__ directories out of the working tree. https://claude.ai/code/session_01JmfhcVugXuQsgxnqZfDjdi
One regression was introduced by the consolidation commit (66e365f), and four were pre-existing issues in master that the differential comparison also surfaced. All five are now covered by fixture tests. 1. fix_hanging_indent: pulled call-argument lambda body left reindent_lambda_body called _walk_lambda_body(normalize=True) unconditionally instead of going through the normalize_lambda_body_sibling_indent wrapper that first checks lambda_body_needs_sibling_indent_normalization. The predicate guarded against normalizing bodies whose root-line indents were already consistent; without it, sibling-indent normalization ran on every outer lambda body, including ones containing nested call-argument lambdas whose body indents were legitimately deeper. 2. format_member_declarations: ate space before inline enum brace find_top_level_brace_initializer returned the first top-level `{` without verifying that the matching `}` ended the declaration. For `enum class Type { PLACE, CANCEL } type;` it returned the `{` of the enum body, treating ` PLACE, CANCEL } type` as a brace initializer. The renderer then re-emitted the type as `Type{...}` with no gap. Now we walk to the matching `}` and require nothing non-whitespace to follow it. 3. format_constructor_initializers: dropped trailing comma In collect_initializers, current_terminated was computed as `endswith(',') and len(parts) == 1`. When the first-line text was `m_a(...), m_b(...),` (two parts plus trailing comma), len(parts) was 2 so the trailing comma was ignored; the next line was appended as a continuation of the second item, dropping the separator. The `len(parts) == 1` clause was wrong — the trailing comma always terminates the last part regardless of how many parts there are. 4. format_constructor_initializers: ternary `:` read as ctor-init previous_signature_start accepted any backwards-balanced-paren line as a signature. For a ternary like ? protocol::select_pain_profile(bpd) the prefix `? protocol::select_pain_profile` was treated as a function name. Now we reject prefixes that don't begin with an identifier, `~` (destructor), or `::`. 5. format_constructor_initializers: block-comment `(SSO):` ate text split_inline_initializer skipped `#` and `//` lines but not block- comment continuations starting with `*`. A line like * std::string has SSO (SSO): short strings are stored matched the `)\s*:` regex and got fed into the initializer collector, mangling the whole comment. Add `*` to the skip set. 311 → 316 tests, all passing. https://claude.ai/code/session_01JmfhcVugXuQsgxnqZfDjdi
Master independently fixed two of the same issues (the inline-enum
brace and the call-argument lambda body) plus added a repair-mode
normalizer for under-indented lambda bodies. Bring all of those in
alongside the five fixes already on this branch.
Conflict resolution in tools/fix_hanging_indent.py: the master fixes
modified normalize_lambda_body_sibling_indent and the parallel
lambda_body_needs_sibling_indent_normalization. This branch had
already consolidated those into a single `_walk_lambda_body` parameterized
on `normalize`. Port master's smart-walker enhancements into that
consolidated walker:
- Track `previous_stripped_lines` (last 5) so we can detect a `{` on
its own line that follows a multi-line lambda signature.
- Recognize access labels (`public:`, `private:`, `signals:`, …) so
they're indented one level shallower than their enclosing block.
- Extend `enters_protected_lambda` to cover the multi-line lambda case:
a solitary `{` after `[&](...)` and an optional `-> Type` return.
- Drop master's redundant duplicate copies of
`normalize_lambda_body_sibling_indent` and
`lambda_body_needs_sibling_indent_normalization` after my walker —
they're now wrappers around `_walk_lambda_body`.
Also keeps master's new `normalize_separate_lambda_body_brace`, which
repairs lambda bodies whose body lines are at the same indent as the
opening `{` (rather than one level deeper).
In tools/format_member_declarations.py both my fix (inline
walk-to-matching-brace in `find_top_level_brace_initializer`) and
master's fix (`find_matching_top_level_brace` helper used in
`parse_member_line`) were merged in. They're both protective and
non-conflicting.
Test count: 311 (before refactor) → 316 (after my five fixture tests) →
322 (after master's six new lambda/access-label tests). All green.
https://claude.ai/code/session_01JmfhcVugXuQsgxnqZfDjdi
…mog' into claude/simplify-refactor-code-L7mog
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
Extracted common utility functions that were duplicated across multiple style enforcement tools into a new shared
_style_utils.pymodule. This eliminates code duplication and provides a single canonical implementation for string/character literal handling, bracket matching, and other parsing utilities.Key Changes
Created
tools/_style_utils.py: New shared module containing:_scan_with_strings(): Core utility for walking text while tracking string/char literal statefind_matching_paren()and backward-compatible aliasfind_matching_paren_in_text()split_top_level_commas()andfind_top_level_equals(): Unified implementations using_split_or_find()is_product_source(),is_table_call(),iter_default_paths(),wrapped_control_header_open(),line_close_position(),opens_simple_call_condition(),is_rhs_standalone_boolean_call_clause(),join_expression_fragments()leading_boolean_op(),trailing_boolean_op(),starts_wrapped_boolean_initializer(),starts_wrapped_boolean_return(),strip_trailing_boolean_op()looks_like_function_signature(),looks_like_direct_init_declaration(),split_parameter_type_and_name(),split_string_literal(),align_signature_args(),args_between_parens(),signature_suffix_is_prototype_at()collect_inline_operator_return_chain(),collect_operator_return_block(),render_operator_return_block()find_matching_bracket(),single_bitwise_or_count(),trailing_bitwise_or(),round_type_width_to_tab_stop()Updated
tools/fix_hanging_indent.py:cpp_expression_splitwith imports from_style_utilsbrace_delta()to use_scan_with_strings()callback patternnormalize_lambda_body_sibling_indent()andlambda_body_needs_sibling_indent_normalization()into unified_walk_lambda_body()functionUpdated
tools/wrap_long_conditions.py:_style_utilsfind_matching_paren(),top_level_boolean_ops(), andline_comment_start()to use_scan_with_strings()callback patternsplit_top_level_commas()to use shared implementationfind_top_level_comparison()function using callback patterninside_wrapped_control_header()andinside_return_operator_block()to use common_scan_back_for_block_origin()helperUpdated
tools/check_hanging_indent.py:_style_utilsmacro_line_mask()logicUpdated
tools/format_member_declarations.py:_style_utilsis_product_source()definition_scan_angle_aware()helperUpdated
tools/format_prototype_blocks.py:_style_utilshttps://claude.ai/code/session_01JmfhcVugXuQsgxnqZfDjdi