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

Add support for triangle indicator in/out sub options #221

Merged
merged 2 commits into from
Sep 10, 2020

Conversation

NsCDE
Copy link
Contributor

@NsCDE NsCDE commented Sep 10, 2020

Hi Thomas. This is my long existing patch 2/3 it messes with the same files as window name patch, so I have split it and sent as separate patch.

What does it do: it adds ability for subpanel launcher triangle indicator to be drawn in 3D sunken in, not just raised out. This means that indicator has new argument - after positive integer, there can be keyword "in" or "out". Absence of this argument means "out" which is visual and configuration parsing backward compatible behaviour.

A small illustration attached: right is default raised triangle, left is sunken in new option.
FvwmButtons-Patch-in-out-detail

Copy link
Member

@ThomasAdam ThomasAdam left a comment

Choose a reason for hiding this comment

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

Hi @NsCDE,

Looks good. One minor suggestion only.

Comment on lines 525 to 529
clear_comma = strchr(indicator_in_out, ',');
if (clear_comma != NULL)
{
*clear_comma = '\0';
}
Copy link
Member

Choose a reason for hiding this comment

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

Maybe:

Suggested change
clear_comma = strchr(indicator_in_out, ',');
if (clear_comma != NULL)
{
*clear_comma = '\0';
}
indicator_in_out[strcspn(indicator_in_out, ",")] = '\0';

@NsCDE
Copy link
Contributor Author

NsCDE commented Sep 10, 2020

Hi,

I have implemented your first suggestion.

But I cannot find the second one in the code. I'm bit disoriented now. Also for size limits ... how many to put for this ... please resolve if you can. :-)

@ThomasAdam ThomasAdam merged commit 0a68f90 into fvwmorg:master Sep 10, 2020
@ThomasAdam
Copy link
Member

Hi @NsCDE,

Not a problem. We'll monitor this for now. Let's see how we get on! Merged now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants