-
Notifications
You must be signed in to change notification settings - Fork 362
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
Make better use of part names #644
base: master
Are you sure you want to change the base?
Conversation
In web browsers, this shows tooltips when the cursor hovers over lines of a part.
I very much like what this is aiming at. I am not that much a fan of the implementation. Let's just make sure we pass the |
Oh I fully agree that the retronaming is not pretty. I've tried to make all changes such that no generators would be changed, given that boxes.py can also be used as a library and I don't know what the API guarantees there are. If I start an alternative version of this where there is a mandatory new_part called at the start of a part rather than at the end (which also makes later steps easier), is there a test suite somewhere that I can run that will execute all generators at least with default configuration? (That'd be helpful because I could make it scream the right way until every move start matches its move end). Ideally, is there something that I can run that asserts that the box outputs did not change? |
And because I'll have to deal with the temptation: Is there a rationale for why the move blocks are not implemented using context managers? (The refactoring necessary to do the labels in the first part will note quite warrant that kind of change, but the temptation will be there). |
I think the main reason is that context managers did not exist in Python when the code was written. Also passing width and height is super annoying. This was necessary in the past as Boxes.py used libcairo. As we now use our own backend, we should be able to calculate the space needed by the pieces and place them correctly accordingly. |
Wait a second. |
Would you care to give me some more pointers as to what you'd like to have simplified? (I can also start refactoring the label assignments based on giving the part name beforehand, but it seems you have more concrete ideas.) |
So far, part names are only used to place red text labels.
This change stores the names in the parts (the constructor argument was previously ignored; an extra argument is added to new_part to account for its typical usage pattern where drawing happens on the last set part, and a new part is created at the end of drawing).
A second commit uses the data that is now stored to make names accessible in browsers' tooltips. Future changes enabled by this would be making parts more easily accessible in DXF as well, eg. to extract them for further processing into 3D models.