Fix for issue #131 - Add func param for menu callback

[andymbody]

Fix for issue #131. ~Will now add param to functions for menu callbacks.~ Also made a couple adjustments to MaskCode.ahk (#213). Added commit to fix #136. Changed args from to the 3 mentioned (for 131). Also fixed a bug that I introduced in my first commit. Also added commit to fix extra CR issue for files with CRLF line terminators.

[Banaanae]

Nice! There's also issue #136, where OnMessage callbacks need wParam, lParam, msg, hwnd. Hopefully this will provide a base to implement this too

[andymbody]

There's also issue..

Ok... will take a look when I can.

[andymbody]

This may be a repeat question, but... how would I change this current PR to include additional commits? I'm not ready to do that, but when I am I will need to know how. I will try to research it, but if there is a quick answer, I would appreciate any tips you may have.

[Banaanae]

So in your repository where you made your current changes, you should be able to make more changes, commit and push.

Sorry if it's not very descriptive :) I can also merge this and then you can make the changes if needed

[andymbody]

So in your repository where you made your current changes, you should be able to make more changes, commit and push.

Sorry if it's not very descriptive :) I can also merge this and then you can make the changes if needed

I think I know what you mean. I haven't researched yet, but i thought about it today and came up with the same possibility... just submit more commits on same branch... right?

I'm not sure how long it will take to add new commits so if you want to merge this now, that would be good. I can always get up to speed with multiple commits another time. Might do that with the next one, just to practice.

[Banaanae]

Also, rather than using * could we use A_ThisMenuItem := "", A_ThisMenuItemPos := "", A_ThisMenu := "" Consider this:

#Persistent
Menu, Tray, Add, My Menu Item, MyHandlerFunc

MyHandlerFunc() {
    MsgBox % A_ThisMenuItem     ; My Menu Item
    MsgBox % A_ThisMenuItemPos  ; 1
    MsgBox % A_ThisMenu         ; Tray
}

Persistent
Tray:= A_TrayMenu
Tray.Add("My Menu Item", MyHandlerFunc)

MyHandlerFunc(A_ThisMenuItem := "", A_ThisMenuItemPos := "", A_ThisMenu := "") {
    MsgBox(A_ThisMenuItem)     ; My Menu Item
    MsgBox(A_ThisMenuItemPos)  ; 11
    MsgBox(A_ThisMenu)         ; Error
}

With * there is a vars are undefined warning

Note 1) A_ThisMenuItemPos returns 1 in v1 and 11 in v2, I think this is because v1 returns the pos of the item ignoring default items, while v2 doesn't. I think we should keep it the same (not alter the variable, keep it at 11), it's more correct.

Note 2) A_ThisMenu fails, because it now returns the menu object instead of Menu name, (there doesn't appear to be any Menu.Name/Menu.Title properties, so it should be ok to leave as is, and maybe make a warning in the future)

[andymbody]

Ok... question...

should we worry about whether there are params already in the param list of the func? What should happen if we find that the func has params, but they have different names (other than A_ThisMenuItem, A_ThisMenuItemPos, or A_ThisMenu)? Will that ever be the case?

Example

Menu, Image, Add, Invert colours of image in clipboard, InvertImageInCb
Menu, Image, Add, Save Clipboard Image as File, SaveCbImageasFile
Menu, Image, Add, Save Clipboard Image as 8-bit PNG, SaveCbImageas8Bit

; Issue #131
; * param needs to be added to funcs

InvertImageInCb() {

    RunWait, magick.exe convert clipboard:myimage -channel RGB -negate -background white -flatten clipboard:, , Hide
    MsgBox, Inverted colors of image in clipboard.
}

SaveCbImageasFile(param1) { ; comment

    RunWait, magick.exe convert clipboard:myimage -channel RGB -negate -background white -flatten clipboard:, , Hide
    MsgBox, Inverted colors of image in clipboard.
}

SaveCbImageas8Bit(param1, param2, param3) {

    RunWait, magick.exe convert clipboard:myimage -channel RGB -negate -background white -flatten clipboard:, , Hide
    MsgBox, Inverted colors of image in clipboard.
}

Should we just add the params you mention and wait to see if an "issue" is posted pointing out that these added params cause an issue?

I'm unfamiliar with Menu (I rarely/never use it), so forgive my ignorance if the answers to these questions are obvious.

[andymbody]

BTW... if i send an additional commit on this PR, I am moving post-convert operations to a dedicated function. This would be any conversion operations that are not within the actual loop of _convertLines(). There were only a couple until recently (placed at the bottom of _convertLines()), but they are growing and I think they deserve their own function/organization for code clarity. The dedicated function will be place just below _convertLines() func. This also prevents code repetition found in both Convert() (with masking) and _convertLines(). Making a call to a dedicated function makes more sense than repeating the code in both places.

Sound ok to you?

[Banaanae]

should we worry about whether there are params already in the param list of the func?

I think we still put the A_ThisMenu* params first, then the user defined params with a warning like ; V1toV2: Update function calls to make [param name] fourth

What should happen if we find that the func has params, but they have different names (other than A_ThisMenuItem, A_ThisMenuItemPos, or A_ThisMenu)? Will that ever be the case?

These are built in variables (like A_ThisFunc and A_ThisHotkey), they used to behave like those, and now they can only be accessed via a menu callback (like A_GuiEvent). We won't need to worry about this happening

Should we just add the params you mention and wait to see if an "issue" is posted pointing out that these added params cause an issue?

Probably not best practice, but this is what I have been doing :) Hopefully if an issue does get reported we might have a better idea on how to handle these situations, (personally, seeing how this code is used in the wild is the best way to fix stuff like this, and in the wild most scripts use labels)

Sound ok to you?

Sounds great

[andymbody]

Added commit for fix of #136. The software showed an error and that the commit did not upload. But it looks like it did. Some of the edited code in this commit changed code in the original commit. So, let me know if I need to resubmit the entire PR.

[Banaanae]

I see the commit too

Do you have plans on fixing the points listed in my prior post? Or should I transfer the comment to an issue

I'll test soon

[andymbody]

Included another commit to prevent extra CR added for files that have line terminators of CRLF. Introduced on 6/15 with UpdateGoto(). (808371a)

I think that's enough commits for this one... Starting to include edits that are unrelated to original commit. But it gave me some practice with updating PR with new commits.