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 '--repl-no-load' option to skip module load in REPL #7578

Merged
merged 8 commits into from
Aug 29, 2021

Conversation

sirlensalot
Copy link
Collaborator

@sirlensalot sirlensalot commented Aug 25, 2021

Fixes #7541

Notes/review items:

  • not supported in GHCJS repl
  • The docs for the existing repl flags, "use this option for the repl", should maybe be improved?

Manually tested on library and executable repl loads. EDIT tests added.

@emilypi
Copy link
Member

emilypi commented Aug 25, 2021

@sirlensalot can you add docs for this please?

@fendor
Copy link
Collaborator

fendor commented Aug 25, 2021

A test-case that proves the feature working would be great!

@sirlensalot
Copy link
Collaborator Author

sirlensalot commented Aug 26, 2021

@sirlensalot can you add docs for this please?

@emilypi I assume this means add docs to cabal-commands.rst; there are minor/incomplete mentions of the repl elsewhere.

Why does cabal-commands.rst only document v2-repl (and for that matter v2-everything)? Is that supposed to be the "true" command doc? I ask b/c I've been confused by this before and been reluctant to read "v2" docs. Likewise, this change is not just for v2-repl so it's even more confusing.

@fendor
Copy link
Collaborator

fendor commented Aug 26, 2021

@sirlensalot v2- commands are the normal, modern commands that you usually use when you use cabal (with a version > 3.0).

E.g. cabal repl is a synonym for cabal v2-repl, they do exactly the same. As such, you did modify the v2-replcommand, when you changed CmdRepl.hs. As a rule of thumb, all modules that start with Cmd*.hs, are v2- commands, or modern versions of their sibling module (e.g. Repl.hs). How can you know? Basically no chance, the final name of the commands on the command line are determined by the argument parsing (I think that happens in Simple.hs but don't quote me on that).

Why do we have both v2-, v1- and un-prefixed commands? v1- are very old and basically deprecated for the normal user, v2- are the new commands which you usually use. And the un-prefixed version are just a default that switched in cabal version 3.0. We have both since cabal changed significantly, a number of breaking changes, and this way, people can keep using v1- commands if they really need to.

@sirlensalot
Copy link
Collaborator Author

A test-case that proves the feature working would be great!

@fendor a breadcrumb of where this test should go would be great. There's tests in cabal-install, Cabal-testsuite, Cabal-test. I was working with the first as IntegrationTests2 seemed like a good fit.

@fendor
Copy link
Collaborator

fendor commented Aug 26, 2021

@sirlensalot It might be a bit complicated, indeed... I'll look into it and report tomorrow what I think is the best place to add tests.

@sirlensalot
Copy link
Collaborator Author

@sirlensalot It might be a bit complicated, indeed... I'll look into it and report tomorrow what I think is the best place to add tests.

@fendor I think I found a good starting place in cabal-testsuite, https://github.com/haskell/cabal/blob/ea830d70144db14175d20f139973aa2865337def/cabal-testsuite/PackageTests/ReplCSources/cabal.test.hs

IntegrationTests2 while familiar/unit-testy doesn't have the infra to work with STDIN so it just hangs.

@sirlensalot
Copy link
Collaborator Author

Tests, docs added; changelog updated.

Copy link
Collaborator

@fendor fendor left a comment

Choose a reason for hiding this comment

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

LGTM!

@emilypi emilypi added the merge me Tell Mergify Bot to merge label Aug 29, 2021
@mergify mergify bot merged commit 785ea62 into haskell:master Aug 29, 2021
@emilypi
Copy link
Member

emilypi commented Aug 29, 2021

@Mergifyio backport 3.6

mergify bot pushed a commit that referenced this pull request Aug 29, 2021
* Add '--repl-no-load' option to skip module load in REPL

* Command docs

* Add tests

* update changelog

Co-authored-by: Stuart Popejoy <[email protected]>
(cherry picked from commit 785ea62)
@mergify
Copy link
Contributor

mergify bot commented Aug 29, 2021

Command backport 3.6: success

Backports have been created

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

Successfully merging this pull request may close these issues.

Support equivalent of stack --no-load with cabal repl
4 participants