Add KiCad 8/9/10 symbol format support - #16
Conversation
Add support for new tokens introduced in KiCad 8, 9, and 10: - show_name and do_not_autoplace in property definitions - embedded_fonts at library and symbol level - in_pos_files, duplicate_pin_numbers_are_jumpers, body_styles at symbol level - unit_name now returns proper AST node - alternate pin names support quoted strings Includes real KiCad 9/10 test files from official kicad-symbols library. Co-Authored-By: Matthias <matthias.chills@gmail.com>
Original prompt from Matthias |
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| = "(" _ type:"embedded_fonts" _ value:bool _ ")" { | ||
| return { type, value: { type: "boolean", value: value === "yes" } }; |
There was a problem hiding this comment.
🔴 embedded_fonts boolean value always resolves to false due to double-wrapping with bool rule
The embedded_fonts rule binds value:bool, where the bool rule (src/symbol.pegjs:514-515) already returns { type: "boolean", value: value === "yes" }. The action then does value === "yes" on this object, which is always false regardless of input.
Root Cause
The bool rule at line 515 captures the raw string "yes"/"no" and transforms it into { type: "boolean", value: true/false }. When embedded_fonts labels this result as value:bool, the identifier value in the action is bound to that object — not the raw string. The comparison value === "yes" compares an object to a string and always yields false.
// Input: (embedded_fonts yes)
// bool returns: { type: "boolean", value: true }
// value === "yes" → false (object !== string)
// Result: { type: "embedded_fonts", value: { type: "boolean", value: false } } ← always false!
This means (embedded_fonts yes) and (embedded_fonts no) both parse to false. The correct fix is to pass through value directly (like hide/bold/italic do at lines 496, 512): return { type, value };.
Note: The pre-existing rules exclude_from_sim (line 67), in_bom (line 110), and on_board (line 115) have the same bug, but those are not part of this diff.
| = "(" _ type:"embedded_fonts" _ value:bool _ ")" { | |
| return { type, value: { type: "boolean", value: value === "yes" } }; | |
| = "(" _ type:"embedded_fonts" _ value:bool _ ")" { | |
| return { type, value }; |
Was this helpful? React with 👍 or 👎 to provide feedback.
| = "(" _ type:"in_pos_files" _ value:bool _ ")" { | ||
| return { type, value: { type: "boolean", value: value === "yes" } }; |
There was a problem hiding this comment.
🔴 in_pos_files boolean value always resolves to false due to double-wrapping with bool rule
Same pattern as embedded_fonts: value:bool returns an object { type: "boolean", value: true/false }, but the action compares it with value === "yes" which is always false.
Root Cause
The in_pos_files rule at line 118-120 uses value:bool then value === "yes". Since bool already transforms "yes"/"no" into { type: "boolean", value: ... }, the subsequent value === "yes" compares an object to a string and always returns false. This means (in_pos_files yes) is incorrectly parsed as false.
The fix is: return { type, value }; to pass the bool result through directly.
| = "(" _ type:"in_pos_files" _ value:bool _ ")" { | |
| return { type, value: { type: "boolean", value: value === "yes" } }; | |
| return { type, value }; |
Was this helpful? React with 👍 or 👎 to provide feedback.
| = "(" _ type:"duplicate_pin_numbers_are_jumpers" _ value:bool _ ")" { | ||
| return { type, value: { type: "boolean", value: value === "yes" } }; |
There was a problem hiding this comment.
🔴 duplicate_pin_numbers_are_jumpers boolean value always resolves to false due to double-wrapping with bool rule
Same pattern: value:bool returns an object, but value === "yes" compares it to a string, always yielding false.
Root Cause
The duplicate_pin_numbers_are_jumpers rule at line 123-125 uses value:bool then value === "yes". Since bool returns { type: "boolean", value: ... }, the comparison value === "yes" is always false. This means (duplicate_pin_numbers_are_jumpers yes) is incorrectly parsed as false.
The fix is: return { type, value };
| = "(" _ type:"duplicate_pin_numbers_are_jumpers" _ value:bool _ ")" { | |
| return { type, value: { type: "boolean", value: value === "yes" } }; | |
| return { type, value }; |
Was this helpful? React with 👍 or 👎 to provide feedback.
| = "(" _ type:"show_name" _ value:bool _ ")" { | ||
| return { type, value: { type: "boolean", value: value === "yes" } }; |
There was a problem hiding this comment.
🔴 show_name boolean value always resolves to false due to double-wrapping with bool rule
Same pattern: value:bool returns an object, but value === "yes" compares it to a string, always yielding false.
Root Cause
The show_name rule at line 156-158 uses value:bool then value === "yes". Since bool returns { type: "boolean", value: ... }, the comparison is always false. This means (show_name yes) is incorrectly parsed as false, and symbol properties that should display their name will not.
The fix is: return { type, value };
| = "(" _ type:"show_name" _ value:bool _ ")" { | |
| return { type, value: { type: "boolean", value: value === "yes" } }; | |
| return { type, value }; |
Was this helpful? React with 👍 or 👎 to provide feedback.
| = "(" _ type:"do_not_autoplace" _ value:bool _ ")" { | ||
| return { type, value: { type: "boolean", value: value === "yes" } }; |
There was a problem hiding this comment.
🔴 do_not_autoplace boolean value always resolves to false due to double-wrapping with bool rule
Same pattern: value:bool returns an object, but value === "yes" compares it to a string, always yielding false.
Root Cause
The do_not_autoplace rule at line 161-163 uses value:bool then value === "yes". Since bool returns { type: "boolean", value: ... }, the comparison is always false. This means (do_not_autoplace yes) is incorrectly parsed as false, and properties that should not be autoplaced will lose that setting.
The fix is: return { type, value };
| = "(" _ type:"do_not_autoplace" _ value:bool _ ")" { | |
| return { type, value: { type: "boolean", value: value === "yes" } }; | |
| return { type, value }; |
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
The PEG grammar (
symbol.pegjs) was missing tokens introduced in KiCad 8, 9, and 10, causing all modern KiCad symbol files to fail parsing with: "Expected 'at', 'effects', or 'id' but 's' found" (triggered by theshow_nametoken).Grammar changes:
show_name,do_not_autoplace, andhideas valid tokens in thepropertyrule'srestalternativesin_pos_files,duplicate_pin_numbers_are_jumpers,body_styles,embedded_fonts, andunit_nametokicad_symbol_elementembedded_fontstokicad_symbol_libtop-level alternativesunit_name: Changed from a silent/discarding rule to one that returns a proper{ type, value }AST nodealternate: Now accepts quoted strings(string / symbol)in addition to bare symbols for pin alternate namesembedded_fonts,in_pos_files,duplicate_pin_numbers_are_jumpers,body_styles,show_name,do_not_autoplaceTest files:
Added two real KiCad 10 symbol files from the official
kicad-symbolslibrary:14528.kicad_sym— simple symbol usingextends, withshow_name/do_not_autoplace/embedded_fonts4001.kicad_sym— complex multi-unit symbol withbody_styles,in_pos_files,duplicate_pin_numbers_are_jumpersAll existing tests pass with no regressions.
Review & Testing Checklist for Human
unit_namerule change doesn't break downstream consumers — previously this rule silently consumed the token; now it returns{ type: "unit_name", value: ... }. Any code processing the symbol AST will now seeunit_namenodes where they were previously absent. The flux-appKiCadModernSymbolParserneeds to handle this.body_stylesvalue shape — returns{ type: "body_styles", value: { type: "string", value: "demorgan" } }. Confirm this is the only known value (KiCad docs suggestdemorganis the only option, but check if others exist).show_name/do_not_autoplacestill parse correctly (existing tests cover standard files, but test against a diverse set if possible).4001.kicad_symfile via the Flux UI and verify it doesn't throw "Could not find any compatible parts in uploaded file."Test Plan
kicad-symbolsrepo:git clone https://gitlab.com/kicad/libraries/kicad-symbols.git /tmp/kicad-symbols.kicad_symfiles from different component categories with the updated parserNotes
symbol-parser.jsdiff is auto-generated from the grammar — focus review onsymbol.pegjschangesflux-appwill updateKiCadModernSymbolParserto consume the new AST fieldsLink to Devin Session: https://app.devin.ai/sessions/103ae3c52c02438aabd3955415051fa5
Requested by: @natarius