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

PCell callbacks invoked on value change. #377

Closed
wants to merge 1 commit into from
Closed

PCell callbacks invoked on value change. #377

wants to merge 1 commit into from

Conversation

rizoschrist-prime
Copy link
Contributor

No description provided.

@klayoutmatthias
Copy link
Collaborator

If you mention the bug number in the commit message (like fixed #376), PR and issue get linked. This way it's easier for me to identify PR and ticket.

@@ -111,7 +111,7 @@ Q_OBJECT
void set_parameters (const std::vector<tl::Variant> &values);

public slots:
void activated (int);
void activated (int i=0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

activated(int) can be replaced in all places by "activated()" simply. Signals can be connected to slots with less parameters.

Copy link
Contributor Author

@rizoschrist-prime rizoschrist-prime Oct 14, 2019

Choose a reason for hiding this comment

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

Hi Matthias, thanks for the review.

As for the purpose of this feature. I have tried to reach you via DM from KLayout forum to ask you for some hints about the changes I wanted to integrate, please it would be much of a help to check them.

Specifically for the purpose of this change, it is easy for someone to "Apply" a corrupted geometry, without continuously checking the input effect for every value. When a UI item loses its focus, or its status changed, 'get_parameters' should be invoked to continuously check the status of all parameters after the possible change. When the user finishes tuning the parameters and is ready to "apply" a layout geometry, the parameters should have already been checked and correspond to a valid geometry, to avoid any corrupted structures.

I thought that it is best practice the signature of a signal to match the signature of the receiving slot, although it is acceptable the slot to have fewer arguments from the activated signal. Boolean signal 'stateChanged' has 'int' in its signature, so I wanted to cover both 'editingFinished' and 'stateChanged' with this implementation.

Should I make a new PR with your proposal, or it is possible to push the changes to the current request?

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