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

I suggest to add few more convenience functions, #7

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

vadrer
Copy link

@vadrer vadrer commented Oct 20, 2018

…which happen to be useful for my scripts,
these are - IsIconic, IsZoomed, OpenIcon, also 'RestoreWindow', which makes window 'restored'

thank you :)

@dk
Copy link
Owner

dk commented Oct 20, 2018

Hi, the idea is good, but I'd like to see a more orthogonal set of functions instead of just a single RestoreWindow call. A Get/Set WindowPlacement xs API, and RestoreWindow/MinimizeWindow/MaximizeWindow written in perl or xs, if needed.

@vadrer
Copy link
Author

vadrer commented Oct 21, 2018

ok, glad to see you approve the idea.
if Get/Set WindowPlacement are ok with you - I'll implement RestoreWindow/MinimizeWindow/MaximizeWindow and then include tests and documentation of those. :)

@dk
Copy link
Owner

dk commented Oct 21, 2018

Yes, like those two. I see though numerous problem with referencing a bare scalar as a WindowPlacement structure. I'd propose to send individual elements (either as a hash or list) to a Set function, and return the similar structure from Get function.

If a structure will be a list, then it will be just a list of integers representing flags,showCmd,ptMinPosition.x,ptMinPosition.y etc - easier for the sake of louse syntax. A much better syntax would be a structure cooked up like this:

{
    flags => WPF_SETMINPOSITION,
    showcmd => SW_HIDE,
    ptMinPosition => [ 5, 25 ],
    .. etc ..
}

nicer but more work needed

@vadrer
Copy link
Author

vadrer commented Oct 21, 2018

those were "raw" low-level functions (Get/Set WindowPlacement ), which then would be used by higher-level functions (RestoreWindow/MinimizeWindow/MaximizeWindow)
Probably "convenience" funcitons "GetWindowPos"/SetWindowPos which are implemented in MFC also easy to implement.

ok, I suggest to rename Get/Set WindowPlacement to _Get/_Set WindowPlacement and provide Get/Set WindowPlacement on top of that, as you've suggested,

@vadrer
Copy link
Author

vadrer commented Oct 23, 2018

...now should be better :)

@dk
Copy link
Owner

dk commented Oct 24, 2018

Yes, almost there! I have some issues more:

  • First, tests, I'd like to see it tested on both 32 and 64 bit systems to make sure pack/unpack is done correctly.
  • Another one is a more rigorous parameter check to SetWindowPlacement - points and rects have to have lengths 2 and 4, respectively; if left unspecified, the function should either die or use default values if these fields are irrelevant in the context (f.ex. if WPF_SETMINPOSITION is unset). And length should not be a part of input, the function should supply it itself, otherwise memory access/corruption can happen, I think.
  • Then, the scalar passed to _GetWindowPlacement is empty, I think one need to pre-allocate the 44 bytes and set the length field correctly.
  • Finally, WPF_SETMINPOSITION and all other constants need to be declared to be exportable not by default though

@vadrer
Copy link
Author

vadrer commented Oct 24, 2018

1,
void main () {
printf("sizeof(long)=%d\n",sizeof(long));
}
outputs 4 on 64 bit windows; (usually I use 32 bit perl and gcc that comes with it, but tried 64 gcc now as well)

2,I haven't introduced "WPF_SETMINPOSITION" yet, so not worried about that yet;

3, same with test - would happily add that too

@dk
Copy link
Owner

dk commented Oct 24, 2018

I think it would be enough to die "panic" unless 4 == length pack L => 0 somewhere either in the sub code or in tests

@vadrer
Copy link
Author

vadrer commented Oct 24, 2018

agree,
I'll make these additions in a few upcoming days, probably at weekend, I think so :)

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