Sindarin extracting core #61
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
adri09070
commented
Mar 23, 2023
adri09070
commented
- moving SindarinDebugSession and SindarinDebugger to Sindarin-Core package
- All methods doing things about core concepts became extension methods of SindarinDebugger
- Setting flags for TODOs
- refactoring methods to use the Sindarin API within Sindarin
- extracting methods from longer methods
- removing unused methods
- renaming unclear methods
- categorizing uncategorized classes
…de the API, formatting code
…on method correctly
Collaborator
Author
|
There's something that bothers me. Iceberg doesn't want to commit the deletion of the class |
This was referenced Mar 24, 2023
Collaborator
Author
|
Argh, I wanted to move documentation to a But now, I have problems and I don't understand why |
Collaborator
Author
|
Ah, here it goes! Don't bother about the last 5 commits |
…thods. Notably, moving core of SindarinDebugger to the trait TSindarin
StevenCostiou
requested changes
Mar 31, 2023
Comment on lines
+46
to
+47
| byte := encoderClass nonExtensionBytecodeAt: pc in: self method. | ||
| ^ #( 243 244 245 252 ) includes: byte |
Member
There was a problem hiding this comment.
Add a comment refering to the method that details the bytecode numbers.
Comment on lines
+72
to
+86
| TSindarin >> basicStep [ | ||
|
|
||
| self debugSession stepInto | ||
| ] | ||
|
|
||
| { #category : #'debug - stepping' } | ||
| TSindarin >> basicStepOver [ | ||
|
|
||
| ^ self debugSession stepOver | ||
| ] | ||
|
|
||
| { #category : #'debug - stepping' } | ||
| TSindarin >> basicStepThrough [ | ||
|
|
||
| self debugSession stepThrough |
Member
There was a problem hiding this comment.
Add a comment explaining the "basic"
Comment on lines
+138
to
+145
| "* 70 */ primitiveNew, | ||
| /* 71 */ primitiveNewWithArg, | ||
| /* 79 */ primitiveNewMethod, | ||
| /* 148 */ primitiveClone, | ||
| /* 160 */ primitiveAdoptInstance" | ||
|
|
||
| self flag: 'These numbers may have changed. This is to be checked'. | ||
| ^ #( 70 71 79 148 160 ) |
Member
There was a problem hiding this comment.
Reference to the VM repo where those primitive numbers are indicated.
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.