Skip to content
This repository was archived by the owner on Nov 11, 2025. It is now read-only.

Implement updating and exporting kicad_parse_gen Schematics for the evaluator#18

Merged
luxas merged 3 commits into
mainfrom
schematic-update-export
Jul 5, 2021
Merged

Implement updating and exporting kicad_parse_gen Schematics for the evaluator#18
luxas merged 3 commits into
mainfrom
schematic-update-export

Conversation

@twelho
Copy link
Copy Markdown
Member

@twelho twelho commented Jul 2, 2021

Bases on #17, crosses the last item off the list in #14. With this the kicad_parse_gen::schematic::Schematic structs are being properly kept track of and can finally be updated using the internal Schematic structs as well as written back to their respective files. Designed so that it should be easy to adapt to the changes in #15 once it gets merged.

kicad_parse_gen has a nasty bug with handling escaped strings, I've added the escape/unescape workaround just for the expression field for now, but since it is more widespread we might want to open an issue for it (and fix it) upstream.

cc @luxas @chiplet

@twelho twelho added the enhancement New feature or request label Jul 2, 2021
@twelho twelho requested review from chiplet and luxas July 2, 2021 17:59

impl<'a, T: Default + PartialEq> OrDefault<'a, T> for T {
fn or_default(self, default: T) -> T {
if self == Default::default() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this check do? Where is the default value defined and for what type?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah some documentation on trait would be great 👍

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documented in 02921b0. Default is a built-in trait that enables fetching the type-specific "default" value, e.g. "" (empty string) for string-like types and 0 for number-like types. I'm using it here to make this generic without having to write an impl for each type comparing against its respective "zero value".

let name = attribute.name.as_str().or_default("Value");
c.update_field(name, &attribute.value.to_string());
c.update_field(
&format!("{}{}", name, "_expr"),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we write back the expression? Isn't the desired truth of that always set by the user in the kicad file?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed it is, but serialization of field values with quotes is broken in kicad_parse_gen (like the TODO I've added below that line states). This means that parsing and serializing back an Eeschema file with quotes while doing no modifications isn't reproducible in the first place, it will mess up escaping of the quotes. Since the whole file needs to be serialized at once (and not just some individual fields), I need to work around the bugs in serializing the expression as well, even though it is read-only internally.

Hardcoding _expr as the suffix isn't the most elegant solution, but it'll do for now IMO before I get around to fixing kicad_parse_gen. Then we can completely get rid of the escape and unescape functions here.


impl<'a, T: Default + PartialEq> OrDefault<'a, T> for T {
fn or_default(self, default: T) -> T {
if self == Default::default() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah some documentation on trait would be great 👍

@luxas
Copy link
Copy Markdown
Member

luxas commented Jul 5, 2021

Thanks

@luxas luxas merged commit 8818fab into main Jul 5, 2021
@twelho twelho deleted the schematic-update-export branch July 5, 2021 13:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants