-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Allows comments in prompt templates #14470
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.
Nice, works pretty well! I don't approve it yet, because it is based on another PR that hasn't been approved yet and I want to avoid accidentally merging the other commit.
Code looks good! I just have a few general thoughts that me might want to address:
Line 7 above:
The syntax highlighting shows the content of line 7 as comment, whereas in reality it isn't. This might be confusing.
Line 6 above:
It doesn't show the comment-like string as a comment, however, due to the similar begin and end delimiters to variables, it highlights this false comment as a variable. This again might be a bit confusing. Maybe it'd be better to choose a different sequence of characters for comments, to avoid this overlap and false highlighting.
I will check this once #14465 is done, as it needs to be tested in combination |
Signed-off-by: Jonas Helming <[email protected]>
21b8c8f
to
b53c2af
Compare
This is actually expected. This was a trade off. {{!-- is a very common pattern in template engines. Also we take less characters away for regular usage in prompts. So if {{!-- occurs somewhere not in the first line, it is indeed treated as a variable. The other issue should be fixed now |
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.
Thanks, looks good to me! 👍
This should be reviewed first: #14465
fixed #14469
What it does
Allows comments in prompt templates
How to test
There are automated tests.
Manual test:
Add comments and observe no effect on:
Check comments are highlighted correctly in the prompt editor:
Review checklist
Reminder for reviewers