-
Notifications
You must be signed in to change notification settings - Fork 2
Simpler loop syntax #124
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
Simpler loop syntax #124
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @rycont, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces significant enhancements to the language's looping constructs by adding new, more intuitive syntax for both count-based and list-based iterations. It also includes a substantial refactoring of the parsing rule definitions, improving the modularity and clarity of the language's grammar. These changes aim to make loop usage more flexible and user-friendly.
Highlights
- New Count-Based Loop Syntax: I've introduced new syntax for count-based loops, allowing users to specify a number of repetitions using patterns like
N번 반복
,반복 N번
, andN번 반복하기
. This includes the addition of a newCountLoop
node (core/node/count-loop.ts
) and a specific error (LoopCountIsNotNumberError
incore/error/loop.ts
) for cases where the loop count is not a number. The parsing rules for these new syntaxes are defined incore/prepare/parse/rule/count-loop.ts
. - Expanded List Iteration Syntax: The existing list iteration syntax has been enhanced with new variations:
목록 의 항목 마다 반복하기
and목록 의 항목 마다 반복
. These provide more flexible and natural ways to iterate over elements in a list, complementing the original반복 목록 의 항목 마다
syntax. The parsing rules for these new forms are now defined incore/prepare/parse/rule/list-loop.ts
. - Parsing Rule Refactoring: To improve code organization and maintainability, I've refactored the parsing rules for different loop types into dedicated files:
core/prepare/parse/rule/count-loop.ts
andcore/prepare/parse/rule/list-loop.ts
. The main rule definition file (core/prepare/parse/rule/index.ts
, formerlycore/prepare/parse/rule.ts
) now imports and aggregates these modular rule sets. - Basic Loop Syntax Clarification: The syntax for the basic, unconditional loop has been clarified with the explicit introduction of
반복하기
followed by a block. This provides a clearer keyword for defining an infinite loop that can be exited with반복 그만
. - Documentation and Test Updates: The language documentation (
docs/language/9. loops.md
) has been updated to reflect the new loop syntaxes and their usage. Comprehensive test cases have been added totest/codes/repetition.yak
andtest/errors/loop.test.ts
to ensure the correct parsing and execution of these new loop constructs and their error handling.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
Deploying dalbit-yaksok with
|
Latest commit: |
c5d6d81
|
Status: | ✅ Deploy successful! |
Preview URL: | https://d0d661cc.yaksok-ts.pages.dev |
Branch Preview URL: | https://simpler-loop-syntax.yaksok-ts.pages.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces new syntax for count-based and list-based loops. The changes include new AST nodes and parsing rules. The implementation is well-structured, refactoring existing loop rules into separate files for better organization.
My review focuses on improving correctness and maintainability. I've suggested adding validation for loop counts to ensure they are non-negative integers, preventing unexpected behavior. I've also recommended improving variable naming in the new parsing rules for better code clarity.
}) | ||
} | ||
|
||
const countNumber = countValue.value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code only checks if the loop count is a NumberValue
, but it doesn't validate if the number is a non-negative integer. This can lead to non-intuitive behavior. Consider adding a check to validate that the count is a non-negative integer and throw a specific error if it's not. This would improve the correctness and user-friendliness of the language.
const list = nodes[1] as Evaluable | ||
const body = nodes[4] as Block | ||
|
||
return new CountLoop(list, body, tokens) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For better code clarity and maintainability, the variable holding the loop count should be named count
instead of list
.
const list = nodes[1] as Evaluable | |
const body = nodes[4] as Block | |
return new CountLoop(list, body, tokens) | |
const count = nodes[1] as Evaluable | |
const body = nodes[4] as Block | |
return new CountLoop(count, body, tokens) |
const list = nodes[0] as Evaluable | ||
const body = nodes[4] as Block | ||
|
||
return new CountLoop(list, body, tokens) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For better code clarity and maintainability, the variable holding the loop count should be named count
instead of list
.
const list = nodes[0] as Evaluable | |
const body = nodes[4] as Block | |
return new CountLoop(list, body, tokens) | |
const count = nodes[0] as Evaluable | |
const body = nodes[4] as Block | |
return new CountLoop(count, body, tokens) |
const list = nodes[0] as Evaluable | ||
const body = nodes[4] as Block | ||
|
||
return new CountLoop(list, body, tokens) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For better code clarity and maintainability, the variable holding the loop count should be named count
instead of list
.
const list = nodes[0] as Evaluable | |
const body = nodes[4] as Block | |
return new CountLoop(list, body, tokens) | |
const count = nodes[0] as Evaluable | |
const body = nodes[4] as Block | |
return new CountLoop(count, body, tokens) |
신규 문법 추가: 횟수 반복
New Syntax Sugar: 리스트 반복