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

WIP: normalize macro #5317

Closed
wants to merge 1 commit into from
Closed

Conversation

WilstonOreo
Copy link
Contributor

syntax tests fail for some reason

@@ -120,6 +120,10 @@ Return the arguments with the minimum (or maximum) value. All arguments must be

Perform a modulo operation, where T is some numeric type.

### `normalize(T, T, T) -> T`

Normalizes a `value` in the range of `minimum` and `maximum` and returns a new value in the range [0,1].
Copy link
Member

Choose a reason for hiding this comment

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

So I assume its normalize(value, minimum, maximum)? IMHO that could be a bit clearer in the description.

IIRC we can not extract the argument names that go with those Ts in the list of arguments :-(

@ogoffart
Copy link
Member

ogoffart commented Jun 1, 2024

Thanks for the PR, and nice that you could get it working.
However, I'm torn whether it is useful to add this builtin.
I don't think this is something one need to use that often, and it is really easy to do it without it with just a few operations.
So i'm thinking we shouldn't continue on this.

The other thing you mention to have foo.clamp(0, 1) or foo.min(bar) might be interresting to fix however (i filled #5328 about it if you want to have a look)

@tronical
Copy link
Member

tronical commented Jun 2, 2024

Same here: I don't feel too strongly about this, but I'm unsure if this is a frequently used function that should be in the standard library - especially as global function.

@WilstonOreo
Copy link
Contributor Author

The initial use case I thought was to determine the relative position in a slider or progress bar.
And in std-widgets style, there are a few cases indeed, but it would save only about 1 line of code for each use :)
Moreover, after reading the source code of the styles, I noticed there are quite a lot of redundancies and I think it would be much better to have templates with a defined interface for each stylable widget instead, e.g.:

export component SliderTemplate {
in property <float> value: 0.0;
in property <float> minimum: 0.0;
in property <float> maximum: 0.0;
out property <float> level: (value - minimum) / (maximum - minimum); // Our normalize function
};

If you want, I can open another issue to track this.

@WilstonOreo WilstonOreo closed this Jun 4, 2024
@ogoffart
Copy link
Member

ogoffart commented Jun 4, 2024

Regarding the code duplication in style, yes, we thought about that.
We are considering interfaces (tracking issue 1870) as well as common base that could even be public. (eg, SliderBase although in that case it's not a base component)

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.

None yet

5 participants