+
Skip to content

expand.c: Add "maximum" and "minimum" to math. #1176

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

Merged
merged 1 commit into from
May 21, 2025

Conversation

pghvlaans
Copy link
Contributor

@pghvlaans pghvlaans commented Feb 21, 2025

This commit would add < (minimum) and > (maximum) to the list of available operations in $[math]. I think this could potentially be used to do some interesting things, such as determining the orientation of a rotating monitor in a portable way.

@ThomasAdam ThomasAdam added the type:enhancement Augmenting an existing feature label Feb 21, 2025
@ThomasAdam ThomasAdam added this to the 1.1.3 milestone Feb 21, 2025
@somiaj
Copy link
Collaborator

somiaj commented Feb 21, 2025

I would like to know more about the use case, since expansion variables aren't really conditionals. To me this sounds more like conditionals should be extended to be able to test this, but unsure if expansion variables are the best place to add some conditional like this.

@somiaj
Copy link
Collaborator

somiaj commented Feb 21, 2025

Also as a [$math.] operation, it seems more reasonable to me that it should return the bigger or smaller of the two values, not just a truth value.

@pghvlaans
Copy link
Contributor Author

I would like to know more about the use case, since expansion variables aren't really conditionals. To me this sounds more like conditionals should be extended to be able to test this, but unsure if expansion variables are the best place to add some conditional like this.

It's currently possible to find out which of two values is greater indirectly by using $[math./.x,y] and seeing if the result is EnvMatch with 0. Maybe someone wants to show a program's MiniIcon in Title, but only if the window's width is over, say, 200. Of course, this approach will stop working if math uses floating values in a future release.

Putting < and > in math struck me as the most logical choice since they compare two numbers.

Also as a [$math.] operation, it seems more reasonable to me that it should return the bigger or smaller of the two values, not just a truth value.

Maybe a separate expansion variable for numerical comparisons? Maximum and minimum could go in as well.

@ThomasAdam
Copy link
Member

I think we need to be a bit careful here...

The DSL in fvwm isn't formal; we're in danger here of creating a reverse-polish engine with $[math.X.Y] without realising it.

The original use-case of introducing these placeholders was to allow for a more direct route to performing simple arithmetic logic, without requiring things like PipeRead to do it.

But there will also come a point where one needs to consider PipeRead anyway.

Whether we allow this PR nor not, is unclear to me.

I do agree with @somiaj though that any gt or lt proposals should return the number and not be a boolean operation.

@somiaj
Copy link
Collaborator

somiaj commented Feb 22, 2025

In terms of an operation on integers, < should just be a short hand for minimum and > for maximum. I think there is a use case for adding more variable testing to the conditionals. Test (Version >= 1.1.0) can do numerical testing, I don't see why we can't also allow numerical testing between environment/infostore variables.

@ThomasAdam
Copy link
Member

In terms of an operation on integers, < should just be a short hand for minimum and > for maximum. I think there is a use case for adding more variable testing to the conditionals.

OK, I can see that. Would you be able to come up with a suitable list?

Test (Version >= 1.1.0) can do numerical testing, I don't see why we can't also allow numerical testing between environment/infostore variables.

This example is employing a "hack"; look at its implementation -- if anything I'd rather see it be converted to be a subset of the math(s) code we're referring to here.

Either way... my point here still stands that we do still need to be careful about how we expect users to use this. Maybe that's out of scope to this PR, I grant you -- but I also don't want to just merge this PR bindly. Sorry, @pghvlaans -- but if I decide I wish to increase the scope of this at the expense of this PR, I shall...

@pghvlaans
Copy link
Contributor Author

Sorry, @pghvlaans -- but if I decide I wish to increase the scope of this at the expense of this PR, I shall...

Oh, sure, no worries.

@pghvlaans pghvlaans changed the title expand.c: Add "greater than" and "less than" to math. expand.c: Add "maximum" and "minimum" to math. Feb 27, 2025
@pghvlaans
Copy link
Contributor Author

Forgot to do the discussed change to maximum and minimum.

Comment on lines 1652 to 1654
The comma in "<x>,<y>" is to allow for a future possibility of using floats.
Maximum (>) returns the greater of two values, and minimum (<) the lesser.
All operations use C integer mathematics. The result of integer division is
Copy link
Collaborator

@somiaj somiaj May 19, 2025

Choose a reason for hiding this comment

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

Move this up to the list of all the other operations in the previous paragraph. The statement is out of place here in talking about floats and integer math.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to previous paragraph, merged in intervening commits, squashed and force pushed.

@ThomasAdam ThomasAdam moved this from PRs to In progress in FVWM3 May 19, 2025
@ThomasAdam ThomasAdam removed this from the 1.1.3 milestone May 19, 2025
@somiaj
Copy link
Collaborator

somiaj commented May 21, 2025

Your merge/force push broke your patch, it now contains all the recent stuff added to main too. The diff should only contain your changes (though unsure where the issue is I'm just going off of what github shows me the commit diff is)

@pghvlaans
Copy link
Contributor Author

pghvlaans commented May 21, 2025

Sorry, brain fart. I'll fix it and push again in a few minutes.

EDIT: Done.

@ThomasAdam ThomasAdam added this to the 1.1.3 milestone May 21, 2025
@ThomasAdam
Copy link
Member

LGTM. I'll wait for @somiaj to confirm.

@somiaj -- if you're happy with this PR, don't wait for me, please press the "Rebase and merge" button...

@somiaj somiaj merged commit f154130 into fvwmorg:main May 21, 2025
8 checks passed
@github-project-automation github-project-automation bot moved this from In progress to Done in FVWM3 May 21, 2025
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Jun 7, 2025
# changes

### Enhancements
* FvwmRearrange: Use -on_screen to tile/cascade all windows on the monitor. by @pghvlaans in fvwmorg/fvwm3#1170
* FvwmRearrange: Add -win_cmd COMMAND option. by @somiaj in fvwmorg/fvwm3#1179
* expand.c: Add "maximum" and "minimum" to math. by @pghvlaans in fvwmorg/fvwm3#1176

### Other Changes
* build: macos: fix path to fribidi by @ThomasAdam in fvwmorg/fvwm3#1174
* fvwm-convert-2.6: fix syntax error by @ThomasAdam in fvwmorg/fvwm3#1177
* style: remove superfluous debug by @ThomasAdam in fvwmorg/fvwm3#1181
* Remove autotools & modernise build CI/CD by @ThomasAdam in fvwmorg/fvwm3#1191
* build: remove go-1.14.0 check by @ThomasAdam in fvwmorg/fvwm3#1195
* Fix crash in FvwmPager and clarify Balloons option. by @somiaj in fvwmorg/fvwm3#1197
* build: modify release version script by @ThomasAdam in fvwmorg/fvwm3#1201
* meson: do a link test for `locale_charset()` by @Kangie in fvwmorg/fvwm3#1203
* Small RandR fixes by @ThomasAdam in fvwmorg/fvwm3#1207
* setup_visible_names: don't ignore when both set by @ThomasAdam in fvwmorg/fvwm3#1208
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:enhancement Augmenting an existing feature
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants
点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载