v1 -> v2 Script Converter

[geek]

Sounds good to me. I think it would also get more exposure and updates would be seamless to the users, right?

I don't actually collect visitor statistics from the wiki, but given that I get almost no contributions from other users I hesitate to claim putting anything on the wiki will lead to more exposure at this point in time.

As far as updates go, I'll have to occasionally connect to the sandbox server and push library updates to it. It will be seamless from the user's side, but it won't always stay up to date with the latest changes on GitHub.

i have no problem with you doing this

Thank you. The Wiki sandbox page https://autohotkey.wiki/cloudahk has been updated with a functional converter + diff view. I may move it to a dedicated page later

[guest3456]

i tried testing it but it wasn't working for me in Chrome, but it worked in Edge

[geek]

I'd chalk it up to a caching problem. The wiki platform (dokuwiki) does not have good cache busting controls, so if you visited the wiki before and after my changes to add the converter feature, you may not receive the updated JavaScript until you force refresh the page (e.g. shift+F5)

[guest3456]

github user @Banaanae has been fixing bugs and adding features, and now has write access for quicker additions. big thank you to him for his contributions

[kashmirLZ]

Download the full repo. Then run the included v2converter.exe file

Aint no exe in there sir

[Banaanae]

Aint no exe in there sir

It was deleted a while ago (I'm not too sure why)
But as long as you have ahk v2 installed running the QuickConvertorV2.ahk should work (we have #Requires which should force that)

[guest3456]

Download the full repo. Then run the included v2converter.exe file

Aint no exe in there sir

think dmtr deleted it because it was redundant. just run the eitherh the v2converter or quickcoverter script with ahkv2. will update op

[andymbody]

I've used this tool a number of times and it get some things right, but struggles with many others (as expected).

What are the options to contributing when we run into weaknesses of this tool?

[guest3456]

What are the options to contributing when we run into weaknesses of this tool?

use github issues to report bugs, or submit pull requests for improvements. any pull requests should also include the corresponding unit tests

[andymbody]

use github issues to report bugs, or submit pull requests for improvements. any pull requests should also include the corresponding unit tests

Thanks... sorry... I see that if I had looked thru this entire thread, this question would have been answered.

[andymbody]

I have taken some time to browse the individual posts in this thread. I have also looked at the code in the different .ahk files included in this project.

I made some observations and am interested in your thoughts (those involved in this project) in regards to these observations.

First... it looks like Convert() within ConvertFuncs.ahk has the main loop for the conversions (correct me if I am wrong).

Treatment of Strings...

While looking thru the main loop, I noticed that quoted strings are not being masked, which may be causing some of the issues I have seen. Or did I miss this area in the script?

When I began my converter (never finished), I began by masking block-comments, line-comments, then quoted strings (in that order, using unique place-holder tags). Then after conversions, restored each tagged string, and handled their conversion separately. Then restored line-comments, block-comments. I'm curious why it is more desirable to mask commas and question marks within strings (near line 546), but not the full-strings themselves (using unique tags for each quoted string or escaped-quotes)?

Maybe I missed the full-string masking?

Would it be worth masking each full-string near the top of the conversion loop, so that things like Msgbox """Msg""" is not mistreated, "clipboard" is not converted to A_Clipboard, "new" in not confused with the New Keyword, and commas and question marks within strings are automatically hidden as a result of the full-string-mask itself (to mention just a few string related issues)?

Treatment of functions and structured code...

Another approach I used was to extract all user-defined functions (each masked with unique placeholders tags) to be processed separately so that variable scope could be considered during the conversion. It also allowed some tracking of these function calls within the scope of the rest of the .ahk file, which could be extended to other .ahk files as well (#Include). Is that something that could be beneficial with this project? I see that this conversion is handled line-by-line from top to bottom (with the exception of peeking at pre-lines etc), so separating entire functions to be processed separately would require some loop code to be re-organized. But would that modular approach be a desired state by anyone involved with this project?

Conclusion...

If the idea of full-string masking is not desired, I will not spend time looking for the best spot within the loop to insert/test it. I don't know of any down-side to this approach and I think it comes with benefits. But there may be things that I have not considered up to this point?

Thoughts?

Andy

[guest3456]

Thoughts?

all of the things you said would be good. but i cant comment on the design because this converter was originally built 13 years ago but user Frankie in the old ahk forums. i simply took his old converter and significantly expanded it to accomodate all the new v2 features. he originally designed it on a line by line basis so i just kept building on top

one of the nice things about have the suite of test cases is that users are free to try to make large scale refactors to add features like you are suggesting. as long as all the tests pass then you are good to go. and as you make changes you can run the test suite and see if anything breaks after each change

i personally dont work on this project much any more. other users are more active contributing, with @Banaanae being the most recent user who has taken up the mantle

[Banaanae]

If the idea of full-string masking is not desired, I will not spend time looking for the best spot within the loop to insert/test it. I don't know of any down-side to this approach and I think it comes with benefits. But there may be things that I have not considered up to this point?

Many issues can benefit from this, but with the way the code is it may be work. If you want to tackle this ConvertFuncs.ahk L154 is probably a good place to start
I personally don't see any issues either, and as Mike said there is suite of tests that will pick up any errors that do arise

that quoted strings are not being masked

They aren't, and you're right that it's causing issues, but with how I've interpreted this it may fix one issue but cause another. For example take this line

Code: Select all

Line := StrReplace(Line, "<>","!=")

It will replace all (non-comment) instances of "<>" with "!=" regardless of whether it should, masking would fix this
Then there's stuff that modifies the string, like L330 (replacing "" with `" in assignment), masking would break this, unless it gets moved to the masking function
I'm also curious to whether masking will effect the param formats used in the convert folder (found at L3271) will be affected by masking?

But would that modular approach be a desired state by anyone involved with this project?

This would definitely improve maintainability

Please make a fork if you're interested in implementing some of these, with the way the projects set up we can add parts of your suggestions and test them against the test suite

[andymbody]

Thanks for both of your replies... After playing with the script a little I see your point about how the current design makes it difficult to accomplish some tasks (as it stands now).

With a slight tweak, I was able to fix Msgbox_ex7 without breaking any thing else. This may have fixed others as well, but since the failure-scripts are not part of the Test.ahk, I won't know until I test each failure manually. Or create a script to test each of the failures automatically (like test.ahk/exe does). I think a separate tester for failed code would be a good thing, unless it is already included, (and I missed it?).

For clarification... Each failed test also has a .ah2 file... is this .ah2 the "desired state" (expected) of the conversion? That's how it looks to me, but I want to make sure before I continue.

Also, I noticed that strings are currently only allowed to have double quotes (not singles). This is quite restrictive in those cases when single quotes make more sense for the current string design. For instance, msgbox_ex7 could have benefitted from this (of course msgbox_ex7 specifically is a very rare [almost never] occurrence, since it did not include %). But with a slight tweak, single quotes could be an option when double quotes are found within the string. But it would also require the test.ahk "expected" states to be updated to allow single quotes. An either/or type of validation check for success. Would that be a welcome tweak? It would allow much better formatting of strings rather than the messy ``" that is required now.

Let me know what you think...

I will wait to submit the change made for Msgbox_ex7 fix until I have tested others (and figure out how to submit on Github - I have no experience with this )

[Banaanae]

failure-scripts are not part of the Test.ahk

afaik test.ahk is more of a legacy file, it existed before the new format with all the .ah1 and .ah2 files. If you're using QuickConvertV2.ahk you can open Settings in the menubar and Turn on Testmode and Include Failing to test failing and existing tests

is this .ah2 the "desired state" (expected) of the conversion?

The .ah2 file is a recommendation on how the converter should convert the script, running both scripts should behave the same (or as close v2 can, sometimes this isn't possible like with A_ThisLabel)
ofc if the conversion works correctly, but doesn't match the .ah2 file we can overwrite the .ah2 file

I noticed that strings are currently only allowed to have double quotes (not singles)

I don't quite follow?
If you mean single quotes in the tests AHKv1 doesn't support them in strings

image.png (15.39 KiB) Viewed 383 times

Or if you mean single quotes in the conversion code, like here

Code: Select all

if InStr(Line, "`"`"") {

That's just me forgetting I can use single quotes

(and figure out how to submit on Github - I have no experience with this )

There's a quick guide at https://github.com/octocat/Spoon-Knife if you need it

[andymbody]

test.ahk is more of a legacy file, it existed before the new format with all the .ah1 and .ah2 files. If you're using QuickConvertV2.ahk...

Ok... great... thanks for the tip

we can overwrite the .ah2 file

I'll keep this in mind... thanks

if you mean single quotes in the conversion code

I meant that the test.ahk/exe does not consider single quotes to be valid conversion. I will see if this is the case for the other method of testing.

There's a quick guide at

Thank you, I will check it out.

[andymbody]

Successful masking of strings... BUT...

I was able to rearrange some of the steps in the main loop so that I could mask the line-strings prior to some steps being performed. But, once that was done, QuickConvertorV2 showed a couple items had been broken.

However, upon review of the items in question, it looks like the current desired-state in .ah2 is incorrect IMO. See example below

As an example here is ClipWait_ex1. Notice in the original desired state, MsgBox shows that the strings should be changed. I feel that is incorrect... do you agree?

Which of these conversions look correct to you?

v1 code

Code: Select all

Clipboard := "" ; Empty the clipboard
Send, ^c
ClipWait, 2
if ErrorLevel
{
    MsgBox, The attempt to copy text onto the clipboard failed.
    return
}
MsgBox, clipboard = %Clipboard%
return

v2 (original desired state - I feel this is incorrect)

Code: Select all

A_Clipboard := "" ; Empty the clipboard
Send("^c")
Errorlevel := !ClipWait(2)
if ErrorLevel
{
    MsgBox("The attempt to copy text onto the A_Clipboard failed.")
    return
}
MsgBox("A_Clipboard = " A_Clipboard)
return

v2 (output of most recent conversion tweak)

Code: Select all

A_Clipboard := "" ; Empty the clipboard
Send("^c")
Errorlevel := !ClipWait(2)
if ErrorLevel
{
    MsgBox("The attempt to copy text onto the clipboard failed.")
    return
}
MsgBox("clipboard = " A_Clipboard)
return

The 2 others that show to be broken after the tweak are
OnClipboardChange_ex1
WinMove_ex2

But the .ah2 file for these also show that the "clipboard" strings should have been altered to "A_Clipboard", which I don't think should be considered a proper conversion.

If you feel that text within strings that matches the name of keywords should be altered, there will be no way to distinguish which should be altered, and which should not. Therefore all keywords will need to be changed regardless of where they are found within the code.

Do you agree?

Andy

[Banaanae]

clipboard is a string, and shouldn't be converted to A_Clipboard. The test is more to demonstrate that the conversion of ClipWait is working, but yes clipboard should stay the same, feel free to overwrite tests as you see fit.

[andymbody]

The test is more to demonstrate that the conversion of ClipWait is working, but yes clipboard should stay the same, feel free to overwrite tests as you see fit.

Ah... good point... I will keep this in mind going forward...

I am keeping track of the changes being made so that I can outline/upload them separately (is that the preferred method?). I am adding comments about the changes being made as I go. I assume this will be helpful to others who browse the code, in case something gets broken but it's not obvious and shows up later. I ask because I do not see these type of comments (change dates, etc) within the code now.

I assume I will also upload the altered .ah2 files as well?

Sorry for being such a noob to this collaboration process. I usually code alone.

Andy

[Banaanae]

I am keeping track of the changes being made so that I can outline/upload them separately (is that the preferred method?)

Yes, we do prefer this as it makes it much easier to pinpoint issues, although I'm not quite sure how you plan to add them to the head repository? I've also since made some changes to the repository, just a heads up.

I ask because I do not see these type of comments (change dates, etc) within the code now.

We do have commits explaining what parts of the code do, I suspect you aren't seeing many about changes because we tends to add things, rather than making large changes to existing code (which don't get me wrong, we appreciate your work). Dates though should be necessary as git tracks when each commit (change) is made
There is also a lot of debugging code available (mostly commented out), this can be very useful, especially when modifying existing code. I'll just note that most of this code is from v2-alpha (when commands still existed) so conversion will be necessary

I assume I will also upload the altered .ah2 files as well?

Yes, conversion has changed and the tests need to reflect this