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 global configuration option for non-interactive cabal init. #5825

Merged
merged 1 commit into from
Jan 15, 2019

Conversation

m-renaud
Copy link
Collaborator

@m-renaud m-renaud commented Jan 4, 2019

Overview

This PR adds support for specifying the non-interactive option for cabal init in ~/.cabal/config with the default value being False. This does not change any default behaviour.

When the configuration file is generated (when it does not exist), it will contain the following line:

-- non-interactive: False

This partially addresses #5696.

Testing

Manually tested. Added non-interactive: False to ~/.cabal/config and ran cabal init:

$ cabal init

Guessing dependencies...

Generating LICENSE...
Warning: unknown license type, you must put a copy in LICENSE yourself.
Generating Setup.hs...
Generating CHANGELOG.md...
Generating myapp.cabal...

Warning: no synopsis given. You should edit the .cabal file and add one.
You may want to edit the .cabal file and add a Description field.

Note that no .hs files (other than Setup) are generated because the default is Library which currently doesn't generate any Haskell files for (at least until #5740 is merged). Also, I'd like to change this to generate an executable by default (see follow-ups section below).

Possible follow-ups (RFC)

  • Change the flag from a negative (non-interactive: False) to a positive (interactive-init: True)
  • Change cabal init to by default generate an Executable or LibAndExe (the more common case, see the "Default to executable" section of Beginner friendly cabal-install CLI #5696 for rationale).
  • Change the default to be non-interactive (cabal init requires no args and generates an executable)

Please include the following checklist in your PR:

  • Patches conform to the coding conventions.
  • Any changes that could be relevant to users have been recorded in the changelog.
  • The documentation has been updated, if necessary.
  • If the change is docs-only, [ci skip] is used to avoid triggering the build bots.

Please also shortly describe how you tested your change. Bonus points for added tests!

@@ -873,6 +914,15 @@ configFieldDescriptions src =
configAllowNewer (\v flags -> flags { configAllowNewer = v })
]

++ toSavedConfig liftInitFlag
(initOptions ParseArgs)
["quiet", "no-comments", "minimal", "overwrite", "package-dir",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm wondering if toSavedConfig would be better if this was an explicit list of what to include in the global config instead of an exclude list. In this case non-interactive is the only thing (at this point) from InitFlags that we want to be able to configure in ~/.cabal/config. Using an exclude list also introduces the possibility of newly added flags sneaking into the global config. Just a thought.

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense. We could also do something like IncludeAll | IncludeWhiteList [FlagName].

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added to my TODO list :)

@m-renaud
Copy link
Collaborator Author

m-renaud commented Jan 4, 2019

Not sure if this needs to be in the changelog since it may be renamed before the 3.0 release. I'll update the changelog when we finalize on naming/behaviour/etc.

@m-renaud m-renaud requested a review from 23Skidoo January 4, 2019 19:03
Copy link
Member

@23Skidoo 23Skidoo left a comment

Choose a reason for hiding this comment

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

LGTM.

@@ -873,6 +914,15 @@ configFieldDescriptions src =
configAllowNewer (\v flags -> flags { configAllowNewer = v })
]

++ toSavedConfig liftInitFlag
(initOptions ParseArgs)
["quiet", "no-comments", "minimal", "overwrite", "package-dir",
Copy link
Member

Choose a reason for hiding this comment

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

Makes sense. We could also do something like IncludeAll | IncludeWhiteList [FlagName].

@23Skidoo 23Skidoo merged commit 5fbd32e into haskell:master Jan 15, 2019
@23Skidoo
Copy link
Member

Merged, thanks!

@23Skidoo
Copy link
Member

I'd change the name to something more intuitive like non-interactive-init though, because

-- non-interactive: False

gives me no clue about what it does.

@m-renaud
Copy link
Collaborator Author

@23Skidoo

I'd change the name to something more intuitive like non-interactive-init though, because

I'd like to refactor/rename it to --interactive-init in the cabal init command line CLI, and change the default value to False. I originally proposed this in the #5696 but there was push back because some folks want the existing "interactive by default" behaviour. With this now available as a global config option it is easy to obtain the old behaviour by setting the following in ~/.cabal/config:

interactive-init: True

In general I find negative flags confusing, both in CLI usage as well as when reading/modifying the code. Now that this is I'll revisit the issue in the issue I linked above.

@23Skidoo
Copy link
Member

Sounds good!

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