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

Add comment statement #199

Merged
merged 4 commits into from
Oct 4, 2023
Merged

Add comment statement #199

merged 4 commits into from
Oct 4, 2023

Conversation

ds-cbo
Copy link
Contributor

@ds-cbo ds-cbo commented Oct 3, 2023

Part of #189

endif
TSFLAGS ?=
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for example: make test TSFLAGS=--debug-build to speed up the testing cycle

Copy link
Collaborator

@dmfay dmfay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the makefile improvements! there are a few incorrect paths in _comment_target; it's pretty self-contained so I'm not too too worried about it but a little more test coverage would also help

grammar.js Outdated Show resolved Hide resolved
grammar.js Outdated Show resolved Hide resolved
grammar.js Outdated Show resolved Hide resolved
grammar.js Outdated Show resolved Hide resolved
grammar.js Outdated Show resolved Hide resolved
grammar.js Outdated
@@ -723,6 +724,59 @@ module.exports = grammar({
),
),

comment_text: $ => $._literal_string,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment text is a reasonably special case imo but consider the common alias($._literal_string, $.literal) pattern here!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I was already wondering how to get a literal string to be visible in the test cases, I hadn't figured out alias yet. Thanks!

ds-cbo and others added 2 commits October 4, 2023 10:18
Co-authored-by: Dian Fay <dmfay@users.noreply.github.com>
@dmfay dmfay merged commit 385aff4 into DerekStride:main Oct 4, 2023
@ds-cbo ds-cbo deleted the add-comment branch October 4, 2023 14:54
@ds-cbo
Copy link
Contributor Author

ds-cbo commented Oct 4, 2023

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants