[OpenSmalltalk/opensmalltalk-vm] macos: Fix --trace for the PharoVM (#231)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[OpenSmalltalk/opensmalltalk-vm] macos: Fix --trace for the PharoVM (#231)

David T Lewis
 

Introduce VMOPTIONLEN that can offset the extra '-' in the
calculations.


You can view, comment on, or merge this pull request online at:

  https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/231

Commit Summary

  • macos: Fix --trace for the PharoVM

File Changes

Patch Links:


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/OpenSmalltalk/opensmalltalk-vm","title":"OpenSmalltalk/opensmalltalk-vm","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm"}},"updates":{"snippets":[{"icon":"DESCRIPTION","message":"macos: Fix --trace for the PharoVM (#231)"}],"action":{"name":"View Pull Request","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/231"}}}</script>
Reply | Threaded
Open this post in threaded view
|

Re: [OpenSmalltalk/opensmalltalk-vm] macos: Fix --trace for the PharoVM (#231)

David T Lewis
 

@krono commented on this pull request.


In platforms/iOS/vm/OSX/sqSqueakOSXApplication.m:

> @@ -166,7 +168,7 @@ - (int) parseArgument: (NSString *) argData peek: (char *) peek
 	/* Options with no arguments */
 
 	NS_DURING;
-		if ([argData compare:  VMOPTIONOBJ("psn_") options: NSLiteralSearch range: NSMakeRange(0,5)] == NSOrderedSame) {
+		if ([argData compare:  VMOPTIONOBJ("psn_") options: NSLiteralSearch range: NSMakeRange(0,VMOPTIONLEN(5))] == NSOrderedSame) {

Side comment: using VMOPTIONOBJ makes no sense for "psn_". It will always have exactly one dash, so using VMOPTIONOBJ (and the otherwise useful VMOPTIONLEN) makes no sense


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/OpenSmalltalk/opensmalltalk-vm","title":"OpenSmalltalk/opensmalltalk-vm","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm"}},"updates":{"snippets":[{"icon":"PERSON","message":"@krono commented on #231"}],"action":{"name":"View Pull Request","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/231#pullrequestreview-106731224"}}}</script>
Reply | Threaded
Open this post in threaded view
|

Re: [OpenSmalltalk/opensmalltalk-vm] macos: Fix --trace for the PharoVM (#231)

David T Lewis
In reply to this post by David T Lewis
 

@zecke commented on this pull request.


In platforms/iOS/vm/OSX/sqSqueakOSXApplication.m:

> @@ -166,7 +168,7 @@ - (int) parseArgument: (NSString *) argData peek: (char *) peek
 	/* Options with no arguments */
 
 	NS_DURING;
-		if ([argData compare:  VMOPTIONOBJ("psn_") options: NSLiteralSearch range: NSMakeRange(0,5)] == NSOrderedSame) {
+		if ([argData compare:  VMOPTIONOBJ("psn_") options: NSLiteralSearch range: NSMakeRange(0,VMOPTIONLEN(5))] == NSOrderedSame) {

Good point. OSX doesn't care about the number of dashes we have. I don't like the code. We should strip the dashes before parsing the arguments but that would have been a bigger change and outside the timebox I had. :(


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/OpenSmalltalk/opensmalltalk-vm","title":"OpenSmalltalk/opensmalltalk-vm","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm"}},"updates":{"snippets":[{"icon":"PERSON","message":"@zecke commented on #231"}],"action":{"name":"View Pull Request","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/231#discussion_r176941835"}}}</script>
Reply | Threaded
Open this post in threaded view
|

Re: [OpenSmalltalk/opensmalltalk-vm] macos: Fix --trace for the PharoVM (#231)

David T Lewis
In reply to this post by David T Lewis
 

@krono commented on this pull request.


In platforms/iOS/vm/OSX/sqSqueakOSXApplication.m:

> @@ -166,7 +168,7 @@ - (int) parseArgument: (NSString *) argData peek: (char *) peek
 	/* Options with no arguments */
 
 	NS_DURING;
-		if ([argData compare:  VMOPTIONOBJ("psn_") options: NSLiteralSearch range: NSMakeRange(0,5)] == NSOrderedSame) {
+		if ([argData compare:  VMOPTIONOBJ("psn_") options: NSLiteralSearch range: NSMakeRange(0,VMOPTIONLEN(5))] == NSOrderedSame) {

No problem, just a side comment to not forget


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/OpenSmalltalk/opensmalltalk-vm","title":"OpenSmalltalk/opensmalltalk-vm","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm"}},"updates":{"snippets":[{"icon":"PERSON","message":"@krono commented on #231"}],"action":{"name":"View Pull Request","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/231#discussion_r176942784"}}}</script>
Reply | Threaded
Open this post in threaded view
|

Re: [OpenSmalltalk/opensmalltalk-vm] macos: Fix --trace for the PharoVM (#231)

David T Lewis
In reply to this post by David T Lewis
 

Agree, we can do better, but this is good enough, we'll see better later.
See also
#136
#181


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/OpenSmalltalk/opensmalltalk-vm","title":"OpenSmalltalk/opensmalltalk-vm","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm"}},"updates":{"snippets":[{"icon":"PERSON","message":"@nicolas-cellier-aka-nice in #231: Agree, we can do better, but this is good enough, we'll see better later.\r\nSee also\r\nhttps://github.com/OpenSmalltalk/opensmalltalk-vm/pull/136\r\nhttps://github.com/OpenSmalltalk/opensmalltalk-vm/pull/181\r\n"}],"action":{"name":"View Pull Request","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/231#issuecomment-377923455"}}}</script>
Reply | Threaded
Open this post in threaded view
|

Re: [OpenSmalltalk/opensmalltalk-vm] macos: Fix --trace for the PharoVM (#231)

David T Lewis
In reply to this post by David T Lewis
 

Merged #231.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/OpenSmalltalk/opensmalltalk-vm","title":"OpenSmalltalk/opensmalltalk-vm","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm"}},"updates":{"snippets":[{"icon":"DESCRIPTION","message":"Merged #231."}],"action":{"name":"View Pull Request","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/231#event-1551922678"}}}</script>