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

[FR] Parser #4

Merged
merged 9 commits into from
May 31, 2022
Merged

[FR] Parser #4

merged 9 commits into from
May 31, 2022

Conversation

aboueleyes
Copy link
Collaborator

@aboueleyes aboueleyes commented Apr 8, 2022

Adding parser

takes a text file and return a list of binary instructions

implented method for converting `R type instructions` parsing.
add quick parser code, not clean, just to test.
also the `labels` part is not finished.
all unit test pass for `R type`
@aboueleyes aboueleyes self-assigned this Apr 8, 2022
@aboueleyes aboueleyes linked an issue Apr 8, 2022 that may be closed by this pull request
7 tasks
ShimaaBetah and others added 3 commits April 8, 2022 17:48
parsing I and J types.
remove `label` stuff, and handle the special instructions for Shifting
and `MOVI`. as they require less registers.
@aboueleyes
Copy link
Collaborator Author

@AhmedNasserG @ShimaaBetah @MohammadOTaha @Abdulaziz-Hassan
can you start testing the parser by adding new tests? to test different instructions?

@aboueleyes aboueleyes linked an issue Apr 24, 2022 that may be closed by this pull request
@aboueleyes aboueleyes changed the title Parser [FR] Parser Apr 24, 2022
@AhmedNasserG AhmedNasserG self-requested a review April 29, 2022 12:33
@Abdulaziz-Hassan
Copy link
Collaborator

Please delete these following files from the PR as the updated ones are in this pull request #9 so they don't conflict
Instruction
RegisterInstruction
JumpInstruction
ImmediateInstruction
Memory
Registers

src/utlis/Parser.java Show resolved Hide resolved
src/utlis/Parser.java Outdated Show resolved Hide resolved
src/utlis/Parser.java Outdated Show resolved Hide resolved
src/utlis/Parser.java Outdated Show resolved Hide resolved
src/utlis/Parser.java Outdated Show resolved Hide resolved
src/utlis/Parser.java Outdated Show resolved Hide resolved
src/utlis/Parser.java Outdated Show resolved Hide resolved
src/utlis/Parser.java Outdated Show resolved Hide resolved
src/utlis/Parser.java Outdated Show resolved Hide resolved
src/utlis/Parser.java Outdated Show resolved Hide resolved
src/utlis/Parser.java Outdated Show resolved Hide resolved
src/utlis/Parser.java Outdated Show resolved Hide resolved
src/utlis/Parser.java Outdated Show resolved Hide resolved
Repository owner locked and limited conversation to collaborators Apr 29, 2022
Repository owner unlocked this conversation Apr 29, 2022
some refactors but didn't finished yet :(
@aboueleyes aboueleyes dismissed stale reviews from Abdulaziz-Hassan and AhmedNasserG April 29, 2022 17:17

not finished yet

@aboueleyes
Copy link
Collaborator Author

Please delete these following files from the PR as the updated ones are in this pull request #9 so they don't conflict Instruction RegisterInstruction JumpInstruction ImmediateInstruction Memory Registers

ok @Abdulaziz-Hassan , if i deleted them they will be deleted from the repo after merging, I will handle them at merging, don't worry :)

fixed JumpInstruction to handle Immediate values
also handle comments and empty lines.
src/utlis/Parser.java Outdated Show resolved Hide resolved
src/utlis/Parser.java Show resolved Hide resolved
src/utlis/Parser.java Outdated Show resolved Hide resolved
@Abdulaziz-Hassan
Copy link
Collaborator

All in all great job 👏 perhaps maybe adding more tests would be better try using parameterized tests see these articles for more info Parameterized Tests in JUnit, Parameterized Tests in JUnit5
you can add multiple instructions to test a wide range of values it may help 😄

@aboueleyes
Copy link
Collaborator Author

aboueleyes commented May 16, 2022

Thank you for your review, Abdulaziz 🤝
for the tests, if I have time I'll do so.

@aboueleyes aboueleyes changed the base branch from main to structure May 16, 2022 03:17
src/utlis/Parser.java Outdated Show resolved Hide resolved
src/tests/ParserTest.java Outdated Show resolved Hide resolved
src/utlis/Parser.java Outdated Show resolved Hide resolved
src/utlis/Parser.java Outdated Show resolved Hide resolved
src/utlis/Parser.java Show resolved Hide resolved
src/utlis/Parser.java Outdated Show resolved Hide resolved
handle negative numbers, refactor some methods, add some tests : )
@aboueleyes aboueleyes merged commit 85db9cb into structure May 31, 2022
@aboueleyes aboueleyes deleted the parser branch June 5, 2022 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ready for revirew
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] jumpy instrution takes value [FR] Parser
5 participants