Open
Conversation
Electro707
changed the title
yaqwsx
reviewed
Jan 8, 2024
Comment on lines
+38
to
+55
| for i in reversed(range(len(components))): | ||
| c = components[i] | ||
| if c.count('-') == 1: # if we have a - for a range (for example R3-R9) | ||
| m = re.match(r'(\w*?)(\d+)-(\w*?)(\d+)$', c) | ||
| try: | ||
| prefix = m.group(1) | ||
| if prefix != m.group(3): # if the first and second prefix don't match, ignore | ||
| continue | ||
| start_n = int(m.group(2)) | ||
| end_n = int(m.group(4)) | ||
| except (IndexError, ValueError): | ||
| # if we either didn't have a full regex match, or the numbers weren't integers somehow? | ||
| continue | ||
| if start_n > end_n: # check that the first number is the lower limit (R6-R2 is not valid) | ||
| continue | ||
| # Replace XY-XZ with [XY, X(Y+1), X(Y+2), ..., X(Z-1), ZX] | ||
| components += [f"{prefix}{i}" for i in range(start_n, end_n+1)] | ||
| components.pop(i) |
Owner
There was a problem hiding this comment.
Just an idea - wouldn't it be cleaner to modify PcbDraw to accept ranges as well for filtering and highlighting? We would cover another use case.
Contributor
Author
There was a problem hiding this comment.
yeah that's fair. I'll look into incorporating it directly in plot
| plot_args += ["--filter", ",".join(components)] | ||
| plot_args += ["--highlight", ",".join(active)] | ||
| plot_args += [boardfilename, outputfile] | ||
| tmp_std = sys.stdout # make a copy of stdout |
Owner
There was a problem hiding this comment.
I understand that Click changes stdout. But I have two questions:
- why it matters?
- the comment "make a copy of stdout" adds no information. I can see you make it. But why? This is what should be in the comment.
Contributor
Author
There was a problem hiding this comment.
- in case there are any temporary debug print statements (yes I know the debugger exists, but at times it's easier for quick debug sessions)
- can iterate in the comment if it's worth keeping
yaqwsx
force-pushed
the
master
branch
2 times, most recently
from
51ee017 to
d2af9d4
Compare
set-soft
reviewed
Mar 27, 2024
| for i in reversed(range(len(components))): | ||
| c = components[i] | ||
| if c.count('-') == 1: # if we have a - for a range (for example R3-R9) | ||
| m = re.match(r'(\w*?)(\d+)-(\w*?)(\d+)$', c) |
Contributor
There was a problem hiding this comment.
Why (\w*?)? This will match things like 1-10, is this desirable?
Contributor
Author
There was a problem hiding this comment.
not intentional. Will replace with [a-zA-Z]
Contributor
There was a problem hiding this comment.
Also force "1 or more", don't allow 0.
Contributor
Author
There was a problem hiding this comment.
Fixed. Also re-based with master
set-soft
added a commit
to INTI-CMNB/KiBot
that referenced
this pull request
Mar 27, 2024
- In the `show_components` and `highlight` options. - So one entry can be something like *R10-R20*. - Can be disabled using the global option `allow_component_ranges`. Idea from yaqwsx/PcbDraw#159
…le R1-R10). Added an stdout copy so that print statement can work
Electro707
force-pushed
the
pr_populate_multiple
branch
from
c822fb2 to
eea9aec
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds the ability for
populateto take a series of components with a start and end reference delimited with a-, and use all the ones in between.For example,
R3-R20will be treated likeR3,R4,R5...R19,R20This PR also makes a copy of stdout and replaces it back. This is because during debugging, I noticed that when the click application is called, stdout no longer points to the terminal's stdout after the execution is completed.