Skip to content

fix: split ambiguous decimals into leading_digits + trailing_digits (#195)#324

Open
rampageservices wants to merge 3 commits intogerbv:developfrom
SourceParts:fix/drill-decimals-split
Open

fix: split ambiguous decimals into leading_digits + trailing_digits (#195)#324
rampageservices wants to merge 3 commits intogerbv:developfrom
SourceParts:fix/drill-decimals-split

Conversation

@rampageservices
Copy link
Copy Markdown
Contributor

@rampageservices rampageservices commented Mar 1, 2026

Summary

Fixes #195. The single decimals field had ambiguous meaning — sometimes digits-before, sometimes digits-after the decimal point — leading to incorrect coordinate parsing for files with non-standard number formats.

  • Replaces decimals + digits_before in drill_state_t with explicit leading_digits + trailing_digits
  • Replaces HA_digits HID attribute with separate HA_leading_digits + HA_trailing_digits
  • Rewrites read_double() into three functions: read_double_buffered() (reads chars into buffer), read_double_impl() (applies format conversion), and read_double() (wrapper)
  • Updates all header parsers (drill_parse_header_is_metric(), drill_parse_header_is_inch(), drill_parse_header_is_metric_comment()) to set both fields correctly
  • Adds test cases for ;FILE_FORMAT=2:4 comment parsing (the original Drill files get scaled incorrectly #195 bug)

Test plan

  • make builds cleanly with no warnings
  • make check — 94 passed, 12 failed (2 new tests pass, all failures are pre-existing)
  • test-drill-195-without-comments passes
  • test-drill-195-with-comments passes
  • No regressions vs develop baseline

Port of Henry Gabryjelski's commit 45f5dc5 + test files from eb56a67, adapted for current develop.

drill_parse_header_is_inch() and drill_parse_header_is_metric() set
state->unit unconditionally, overwriting user-supplied unit settings
even when autodetect is disabled (state->autod == 0).

The M-code handlers (DRILL_M_IMPERIAL, DRILL_M_METRIC) already guard
their unit assignments with if(state->autod). Apply the same guard to
the header parsing functions for consistency.

Fixes gerbv#241
…gits (gerbv#195)

The single `decimals` field had ambiguous meaning: sometimes it stored
digits-before-decimal (for trailing zero suppression), sometimes
digits-after-decimal (for leading zero suppression / scale factor).
This caused incorrect coordinate parsing for drill files with
`;FILE_FORMAT=2:4` comments, where the leading vs trailing digit count
matters for correct zero suppression.

Replace `decimals` and `digits_before` with explicit `leading_digits`
and `trailing_digits` fields in drill_state_t.

Changes:
- Struct: replace decimals + digits_before with leading_digits +
  trailing_digits
- HID attributes: replace HA_digits with HA_leading_digits +
  HA_trailing_digits (fixes attribute round-trip)
- read_double(): split into read_double_buffered() (character reader),
  read_double_impl() (format conversion), and read_double() (wrapper).
  New signature takes leading_digits + trailing_digits instead of
  single decimals parameter
- All call sites updated (~15 locations)
- Header parsers (metric, inch, metric_comment) set both fields
- .gvp test file updated for new attribute names
- Add test files and enable test-drill-195-with-comments

Port of henrygab commits 45f5dc5 and eb56a67.
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.

Drill files get scaled incorrectly

1 participant