Skip to content

Replace GRAMMAR_INFORMATIVE by INFORMATIVE_GRAMMAR #3860

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

Merged
merged 2 commits into from
Jul 2, 2024

Conversation

Bolpat
Copy link
Contributor

@Bolpat Bolpat commented Jun 27, 2024

Because GRAMMAR_INFORMATIVE starts with GRAMMAR, there are stray ones included in the grammar spec.

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @Bolpat! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

@Bolpat
Copy link
Contributor Author

Bolpat commented Jun 27, 2024

I just checked the prospect Grammar page and it looks good.

@Bolpat
Copy link
Contributor Author

Bolpat commented Jun 27, 2024

@ntrel or @thewilsonator, one of you might take a quick look at this. Thanks in advance 😄

@thewilsonator
Copy link
Contributor

Why is this necessary or desired?

@dkorpel
Copy link
Contributor

dkorpel commented Jun 28, 2024

Why is this necessary or desired?

To remove blocks like this from the main D grammar:

image

@Bolpat
Copy link
Contributor Author

Bolpat commented Jun 28, 2024

@thewilsonator, there is a little D program that collects all blocks starting with $(GRAMMAR and adds them to the D Grammar page. The issue is that $(GRAMMAR_INFORMATIVE starts with $(GRAMMAR and is collected as well. I thought about changing the script to do a lookahead and make sure GRAMMAR isn’t followed by another word character, but honestly, I didn’t understand the code, and IMO, it would have required some non-trivial change, as the find operation is in a UFCS chain. So I changed the name of the DDoc macro instead.


Before, the GRAMMAR_INFORMATIVE made D Grammar have stray explanation grammar blocks:

grafik

With this change, using INFORMATIVE_GRAMMAR, those aren’t included anymore:

grafik

The second block of old is from here, where it is part of an explanation that only makes sense in context:

grafik

@thewilsonator
Copy link
Contributor

thewilsonator commented Jun 28, 2024

Surely you can do

-            enum grammarKey = "$(GRAMMAR";
+            enum grammarKey = "$(GRAMMAR";

then. Or is that intended to pick up "$(GRAMMAR_SUMMARY" too?

@Bolpat
Copy link
Contributor Author

Bolpat commented Jun 28, 2024

Surely you can do

-            enum grammarKey = "$(GRAMMAR";
+            enum grammarKey = "$(GRAMMAR";

then.

Is this a joke that I don’t get? Those two lines are exactly the same.

Or is that intended to pick up "$(GRAMMAR_SUMMARY" too?

GRAMMAR_SUMMARY is only used in one place. It’s likely that it’s being picked up, but doesn’t actually do anything when it’s picked up.

@thewilsonator
Copy link
Contributor

Is this a joke that I don’t get?

Nope, that was supposed to be

-            enum grammarKey = "$(GRAMMAR";
+            enum grammarKey = "$(GRAMMAR ";

my bad.

@Bolpat
Copy link
Contributor Author

Bolpat commented Jun 28, 2024

Is this a joke that I don’t get?

Nope, that was supposed to be

-            enum grammarKey = "$(GRAMMAR";
+            enum grammarKey = "$(GRAMMAR ";

my bad.

Most definitely, a space or newline could follow. Possibly also something else. I’ll keep it as-is for now. The Right Thing would be to check for the word ending, but that’s non-trivial.

@Bolpat
Copy link
Contributor Author

Bolpat commented Jun 28, 2024

@ntrel or @thewilsonator, merge?

@Bolpat
Copy link
Contributor Author

Bolpat commented Jul 2, 2024

@thewilsonator, merge?

@dkorpel dkorpel merged commit 73f51d1 into dlang:master Jul 2, 2024
2 checks passed
@Bolpat Bolpat deleted the InfoGrammar branch July 3, 2024 12:10
@Bolpat Bolpat mentioned this pull request Jul 4, 2024
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.

4 participants