Skip to content

fix(protogen): add correct cell positioners for struct and union type sheet parsing#372

Open
Kybxd wants to merge 1 commit intomasterfrom
positioner-for-structs-and-unions
Open

fix(protogen): add correct cell positioners for struct and union type sheet parsing#372
Kybxd wants to merge 1 commit intomasterfrom
positioner-for-structs-and-unions

Conversation

@Kybxd
Copy link
Copy Markdown
Collaborator

@Kybxd Kybxd commented Mar 24, 2026

Problem

When parsing struct type and union type sheets (e.g., #StructDefault, #UnionDefault), the error messages reported incorrect cell positions (NameCellPos / TypeCellPos). For example:

  • In a struct type sheet with vertical layout, a field at row 6 was incorrectly reported as D1 / D2 instead of A6 / B6.
  • In a union type sheet, a field at row 228 column D was incorrectly reported as A228 instead of D228.

This made it difficult for users to locate the actual problematic cell in the Excel file.

Root Cause

  • Struct type sheets lay out fields vertically (one field per row), but the default positioner treated the cursor as a column index.
  • Union type sheets embed field name/type/note in multi-line cells within each value row, but had no positioner to correctly map cursor to the actual Field column.

Solution

  • Added verticalPositioner for struct type sheets: maps (headerRow, cursor)(dataRow + cursor, headerRow), correctly translating field index to data row position.
  • Added unionFieldPositioner for union type sheets: maps cursor to the actual column starting from Field1, at the specific union value row.
  • Added findFieldStartCol helper to locate the Field1 column index in the header row, supporting varying header layouts (with/without Number, Type columns).
  • Both positioners work correctly with transposed tables.

Tests

Added comprehensive unit tests in sheet_mode_test.go covering:

  • verticalPositioner with normal and transposed tables
  • unionFieldPositioner with normal and transposed tables
  • findFieldStartCol with various header layouts including transposed tables

@Kybxd Kybxd requested a review from wenchy March 24, 2026 08:51
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.98%. Comparing base (bc70718) to head (8300437).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #372      +/-   ##
==========================================
+ Coverage   73.91%   73.98%   +0.06%     
==========================================
  Files          87       87              
  Lines        8664     8686      +22     
==========================================
+ Hits         6404     6426      +22     
  Misses       1700     1700              
  Partials      560      560              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

protogen: panic on wrapDebugErr of v0.15.1

1 participant