Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for how config works #42

Merged
merged 1 commit into from
Sep 16, 2018

Conversation

MrSurly
Copy link
Contributor

@MrSurly MrSurly commented Sep 7, 2018

Consider a Config field for a component is -bat_pwr,+ext_pwr,+dual_pwr,+kitchen_sink

Let's ignore the exclude (bat_pwr). This component is present in multiple variants; ext_pwr, dual_pwr, kitchen_sink

Given the current code below, and assuming we selected ext_pwr:

    result = True
    for opt in opts:
        #options that start with '-' are explicitly removed from certain configurations
        if opt.startswith('-') and opt[1:].lower() == self.prefs.pcbConfig.lower():
            result = False
            break
        if opt.startswith("+"):
            result = False
            if opt[1:].lower() == self.prefs.pcbConfig.lower():
                result = True

This means that the result will be False for this component because it loops through opts multiple times. The last opt is +kitchen_sink, but since it sets result to False, and the following if statement fails (because we're using ext_pwr), this component is not fitted.

Proposed change:

included = not bool(opts) #included by default if no options
excluded = False

config = self.prefs.pcbConfig.lower()
for opt in opts:
    #options that start with '-' are explicitly removed from certain configurations
    if opt.startswith('-') and opt[1:].lower() == config:
        excluded = True
        break
    if opt.startswith("+") and opt[1:].lower() == config:
        included = True
        break

result = included and not excluded
  • included is set to True if opts is empty, else it's False
  • Any hit on the iterate breaks, since pcbConfig is only a single value
  • Edge cases where a config includes the same variant twice, then the first variant that matches will take precedence. Arguably, this should be an error condition.

@SchrodingersGat
Copy link
Owner

@MrSurly this makes sense, thanks for the contribution :)

@SchrodingersGat SchrodingersGat merged commit 7cc4597 into SchrodingersGat:master Sep 16, 2018
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.

2 participants