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 languages extension groups to grammar #9584

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

philderbeast
Copy link
Collaborator

@philderbeast philderbeast commented Jan 2, 2024

Follow up on #9580. Shows the grammar of language extensions that before was showing as optcommalist TODO. Move these and default-language to its own section named "Language Extensions" and renames the "Field Syntax Reference" to "Cabal Package Syntax". In the table of contents put these in a new topmost SYNTAX REFERENCE section.

I first tried all languages extensions but the MathJax HTML renderer couldn't handle it; all of the extension rendered but the braces were drawn with interrupted lines. I upgraded to MathJax 3 and went back to using the SVG renderer. I used a different color for the math (same blue color we use for the titles in the docs). To help render wide equations, I allow the docs to render full width.

Template Β: This PR does not modify cabal behaviour (documentation, tests, refactoring, etc.)

Include the following checklist in your PR:

To test:

$ make doc/buildinfo-fields-reference.rst --always-make

Please review the rendered docs, the sections in "SYNTAX REFERENCE".

@philderbeast philderbeast marked this pull request as draft January 2, 2024 19:12
@philderbeast philderbeast force-pushed the grammar/languages-extension-groups branch 2 times, most recently from a7cd690 to 2ea75b4 Compare January 2, 2024 19:17
@haskell haskell deleted a comment from mergify bot Jan 2, 2024
@haskell haskell deleted a comment from mergify bot Jan 2, 2024
@philderbeast philderbeast force-pushed the grammar/languages-extension-groups branch 7 times, most recently from 8fe3e6f to 28d3d5f Compare January 2, 2024 21:01
@phadej
Copy link
Collaborator

phadej commented Jan 3, 2024

The extension syntax is not restricted to known extensions, listing all the extensions is IMO not appropriate, the syntax description is the wrong place. (Same can be said about default-languages as well, but there are only few, so it's not as bad).

@philderbeast philderbeast force-pushed the grammar/languages-extension-groups branch from a82555b to aa152cb Compare January 5, 2024 18:44
@philderbeast
Copy link
Collaborator Author

@phadej I took on what you said about default-extensions and default-languages and moved these to their own section in the docs (at the bottom of the table of contents).

@philderbeast
Copy link
Collaborator Author

We have a "cabal package" syntax reference but we don't have a "cabal project" syntax reference.

@philderbeast philderbeast force-pushed the grammar/languages-extension-groups branch from f622da1 to aa152cb Compare January 5, 2024 19:52
instance Described Extension where
describe _ = REUnion
[ RENamed "enable-extension" reKnownExtension
, RENamed "disable-extension" reDisableExtension
Copy link
Collaborator

Choose a reason for hiding this comment

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

I repeat myself: this is wrong. The grammar accapts any tokens. The extensions specified may be anything, something which particular Cabal version doesn;t know about. The list of extensions is not part of .cabal file specification.

Listing all known extensions in the grammar specification is a bad idea.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How about something like this with its "unknown-extension"?

instance Described Extension where
    describe _ = REUnion
        [ reEnableExtension
        , reDisableExtension
        , reUnknownExtension
        ]

reEnableExtension :: GrammarRegex a
reEnableExtension = RENamed "enable-known-extension" reKnownExtension

reDisableExtension :: GrammarRegex a
reDisableExtension = REUnion ["No" <> reEnableExtension]

reUnknownExtension :: GrammarRegex a
reUnknownExtension = RENamed "unknown-extension" RETodo

Copy link
Collaborator

@phadej phadej Jan 8, 2024

Choose a reason for hiding this comment

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

No. Grammar doesn't care about extension names. They don't belong there.

The KnownExtension is an optimization: a constructor tag (machine integer) is better than a String (or Text). Perfectly, Cabal won't even have those exposed at all (for example it would allow smoother GHC-updates, as adjusting KnownExtension currently is a breaking change, but it would be better if it wasnt).

If people want extension list, add a link to the GHC manual.

EDIT: AFAIK the only place in Cabal which cares about extension names is cabal check feature which is a "linter" for .cabal files.

@philderbeast philderbeast force-pushed the grammar/languages-extension-groups branch 3 times, most recently from e8f79fc to 5418a43 Compare January 6, 2024 14:39
@philderbeast philderbeast marked this pull request as ready for review January 6, 2024 20:29
@philderbeast philderbeast force-pushed the grammar/languages-extension-groups branch from bd7f24f to bd85c5e Compare January 7, 2024 18:54
@philderbeast philderbeast force-pushed the grammar/languages-extension-groups branch from bd85c5e to 1fbdbf3 Compare January 8, 2024 14:43
Copy link
Collaborator

@geekosaur geekosaur left a comment

Choose a reason for hiding this comment

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

I agree with @phadej: extensions are in ghc's wheelhouse, cabal should not know about them (we have occasional discussions about why cabal knows about them at all instead of using ghc --supported-languages, although I don't think it's in a ticket). They certainly don't belong in the grammar.

@philderbeast
Copy link
Collaborator Author

Regarding "extensions shouldn't be in the grammar"; the intention with this change is to improve our documentation and to do that without altering the grammar. If the minor encroachment on the grammar bothers you, I could reduce it further.

As a reader of the docs, I find the red TODO rather glaring and off-putting.

image

@Mikolaj
Copy link
Member

Mikolaj commented Jan 15, 2024

Yes, I think it makes sense to both correct/complete the documentation and minimize it, in particular, so that we don't have to update it as new extensions are added (until we manage to get rid of the need to add them).

@phadej
Copy link
Collaborator

phadej commented Jan 15, 2024

to do that without altering the grammar.

Describe functionality is used to test real (Parsec) parsers. The TODOs are TODOs, but if specified the grammars should be correct. See UnitTests.Distribution.Described

o improve our documentation

The default-extensions documentation is https://cabal.readthedocs.io/en/stable/cabal-package.html#pkg-field-default-extensions, not in grammar. The picture you link actually has a link Documentation of default-extensions, which says

A list of Haskell extensions used by every module. These determine corresponding compiler options enabled for all files. Extension names are the constructors of the Extension type. For example, CPP specifies that Haskell source files are to be preprocessed with a C preprocessor.

(with unfortunately broken link) I.e. If you want to improve the documentation, do it in the right place. Correcting the link to https://hackage.haskell.org/package/Cabal-syntax/docs/Language-Haskell-Extension.html#t:KnownExtension should be sufficient. It's not Cabal's job to classify the extensions (GHC manual does, and having link there would be nice too, i.e. to https://downloads.haskell.org/ghc/latest/docs/users_guide/exts.html)

@Mikolaj Mikolaj removed their request for review April 4, 2024 18:14
- Add space between .. math:: and content in template
- Add disable extension
- Match descriptions with GHC manual
- Satisfy fourmolu (somewhat)
- Split and rename the syntax
- Remove most GHC grammar except language
- Remove test-suite fields from GHC syntax
- Remove package description fields from GHC syntax
- Remove build info fields from GHC syntax
- Get rid of hs-string, put disable-extension first
- Remove everything but language extensions
- Combine recipes with doc/%-syntax.rst pattern rules
- Make syntax its own section
- Partition build info fields
- Add lib to Cabal-syntax-docs
- Add GHC build info fields to ghc-syntax
- Show ghc-syntax as a list of links
- Update make recipe for build info and user guide
@philderbeast philderbeast force-pushed the grammar/languages-extension-groups branch from 1fbdbf3 to 24bfa09 Compare September 7, 2024 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants