-
-
Notifications
You must be signed in to change notification settings - Fork 313
feat: support conditional logic for string in -when command #826
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
Conversation
Summary of ChangesHello @ChangeSuger, 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! 此拉取请求解决了 Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe 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 by creating a comment using either
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 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. 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
|
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
你好,感谢你为项目增加在 when 指令中进行字符串比较的功能。这是一个非常有用的增强。
代码的整体思路是正确的,通过在 getValueFromStateElseKey 中为字符串类型的值包裹引号,使其能被 angular-expressions 正确解析。
我在 scriptExecutor.ts 中发现了一个潜在的问题。当前的变量解析逻辑可能会将包含字母的字符串字面量(例如 'some_string')错误地识别为变量,导致条件判断不符合预期。此外,对于变量名和布尔值周围的空格处理也存在一些可以改进的地方。我在代码中留下了具体的修改建议,希望能帮助你完善这个功能。
总体来说,这是一个很棒的改进,稍作调整后会更加健壮。
| .map((e) => { | ||
| if (e.match(/[a-zA-Z]/)) { | ||
| if (e.match(/true/) || e.match(/false/)) { | ||
| if (e.match(/^(true|false)$/)) { | ||
| return e; | ||
| } | ||
| return getValueFromStateElseKey(e, true); | ||
| return getValueFromStateElseKey(e, true, true); | ||
| } else return e; | ||
| }) |
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.
你好,感谢你的贡献!
当前 -when 指令的解析逻辑存在一些问题,可能会导致意外的行为:
- 字符串字面量误判:当一个字符串字面量(如
'white.png')包含英文字母时,e.match(/[a-zA-Z]/)会返回true,导致它被错误地当作变量处理,而不是字符串本身。这会破坏字符串比较的逻辑。 - 变量名识别不全:
/[a-zA-Z]/这个正则表达式无法正确识别以下划线_或美元符号$开头的合法变量名(例如_myVar)。 - 布尔值和变量名匹配问题:
e.match(/^(true|false)$/)和getValueFromStateElseKey(e, ...)直接作用于未经trim()处理的字符串e。如果表达式中有额外的空格(如a == true),e可能是' true ',会导致匹配或变量查找失败。
为了解决这些问题,我建议重构这部分代码:
- 首先检查并处理字符串字面量。
- 然后使用更准确的方式来识别可能是变量或关键字的 token。
- 在进行所有检查和变量查找之前,先对 token 进行
trim()操作。
这样可以使条件表达式的解析更加健壮和可预测。
| .map((e) => { | |
| if (e.match(/[a-zA-Z]/)) { | |
| if (e.match(/true/) || e.match(/false/)) { | |
| if (e.match(/^(true|false)$/)) { | |
| return e; | |
| } | |
| return getValueFromStateElseKey(e, true); | |
| return getValueFromStateElseKey(e, true, true); | |
| } else return e; | |
| }) | |
| .map((e) => { | |
| const trimmed = e.trim(); | |
| if ( | |
| (trimmed.startsWith("'") && trimmed.endsWith("'")) || | |
| (trimmed.startsWith('"') && trimmed.endsWith('"')) | |
| ) { | |
| return e; | |
| } | |
| if (trimmed.match(/^[a-zA-Z_$]/)) { | |
| if (trimmed === 'true' || trimmed === 'false') { | |
| return e; | |
| } | |
| return getValueFromStateElseKey(trimmed, true, true); | |
| } | |
| return e; | |
| }) |
|
请忽略上一条评论,可能是我误解了 Redux Devtools 的展示导致。 |
67e6f68 to
e591cd9
Compare
支持在 when 指令中进行字符串比较
背景 & 思路
发现在
setVar支持设置字符串变量的情况下,居然不支持在 -when 指令中进行字符串比较,这使得一些条件控制逻辑的书写会特别麻烦甚至写不出来。另外目前已经支持了用户输入,相信根据用户输入进行比较的诉求也是有的。因此研究了下目前的条件执行的实现,并尝试支持字符串比较。目前条件执行依赖 angular 的 compile,下面的条件执行语句实际输入 compile 为
Camille==Enana,实际认为两侧均为变量,但都没有定义,最后变成undefined==undefined,结果为true。实际测试发现,如果将输入调整为
"Camille"=="Enana",就可以正确的输出结果false。因此支持字符串比较的思路如下:'或"包裹;"包裹(考虑到字符串中可能存在",需要将其替换为\\")/** * 取不到时返回 {key} */ export function getValueFromStateElseKey(key: string, useKeyNameAsReturn = false, quoteString = false) { const valueFromState = getValueFromState(key); if (valueFromState === null || valueFromState === undefined) { logger.warn('valueFromState result null, key = ' + key); if (useKeyNameAsReturn) { return key; } return `{${key}}`; } + // 用 "" 包裹字符串,用于使用 compile 条件判断,处理字符串类型的变量 + if (quoteString && typeof valueFromState === 'string') { + return `"${valueFromState.replaceAll('"', '\\"')}"`; + } return valueFromState; }测试用例
这是自测使用的用例,直接复制粘贴到
start.txt就可以测试。可能存在考虑不足的点,欢迎指正。