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 :info command in Eval plugin #1948

Merged
merged 2 commits into from
Jun 21, 2021
Merged

Conversation

akrmn
Copy link
Contributor

@akrmn akrmn commented Jun 20, 2021

99% of the code is copied from GHCi.UI.info and the functions called from there. Ideally this would be exported from GHCi.UI but sadly that's not the case.

Note that the tests commit is marked as [WIP] because, when running the tests on my machine, I keep getting the instances in different orders. I haven't been able to figure out the source of nondeterminism.

This should eventually close #402

Copy link
Collaborator

@pepeiborra pepeiborra left a comment

Choose a reason for hiding this comment

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

Lgtm, thanks Moises!

@pepeiborra pepeiborra added the merge me Label to trigger pull request merge label Jun 20, 2021

import InfoUtil (Eq, Ord, Foo)

-- >>> :info Foo
Copy link
Contributor Author

@akrmn akrmn Jun 20, 2021

Choose a reason for hiding this comment

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

I ended up changing these tests to work on a custom Foo (and Bar) datatype defined in InfoUtil, since using Bool meant way too many instances and potentially broken tests when new instances get added

@@ -0,0 +1,3 @@
packages:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved the test/testdata/cabal.project file one directory up, so I could define another library info-util. This was because :info prints the origin comment (-- defined in "...") using the full path to the file for modules in the same package, which wouldn't be portable.

import Prelude (Eq, Ord)

data Foo = Foo1 | Foo2
deriving (Eq, Ord)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ended up dropping the Show instances since they would still come out in different orders sometimes. Thankfully Eq, Ord and Baz are enough to show the difference between :info and :info!

mb_stuffs <- mapM (GHC.getInfo allInfo) names
let filtered = filterOutChildren (\(t,_f,_ci,_fi,_sd) -> t)
(catMaybes mb_stuffs)
return $ vcat (intersperse (text "") $ map pprInfo filtered)
Copy link
Collaborator

Choose a reason for hiding this comment

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

You could always sort the filtered children before printing to ensure that the output is deterministic

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, the problem is the [ClsInst] part of the tuple returned by GHC.getInfo. Sadly, ClsInst doesn't have an Ord instance, and I didn't want to impose any custom ordering

@akrmn
Copy link
Contributor Author

akrmn commented Jun 20, 2021

hmm, seems like :info outputs a different format on GHC 8.8.4 vs 8.10.5. Perhaps a golden test isn't the best way to test that command? any ideas @pepeiborra @jneira?

@jneira
Copy link
Member

jneira commented Jun 20, 2021

mmm maybe different files for ghc version (I hope only will be two)?
or switch the test to look for the significant strings

@@ -107,11 +108,56 @@ tests =
]
, goldenWithEval "Works with NoImplicitPrelude" "TNoImplicitPrelude" "hs"
, goldenWithEval "Variable 'it' works" "TIt" "hs"

, testGroup ":info command"
[ testCase ":info reports type, constructors and instances" $ do
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As suggested by @jneira, these tests now only look for the relevant infix strings (or their absence)

@jneira
Copy link
Member

jneira commented Jun 21, 2021

99% of the code is copied from GHCi.UI.info and the functions called from there. Ideally this would be exported from GHCi.UI but sadly that's not the case.

maybe it worths open a ticket in ghc asking for make it public?

@mergify mergify bot merged commit 0d9ba15 into haskell:master Jun 21, 2021
@akrmn akrmn deleted the eval-info-cmd branch June 21, 2021 16:00
@jneira
Copy link
Member

jneira commented Jun 21, 2021

@akrmn many thanks for this useful feature

@jneira
Copy link
Member

jneira commented Jun 22, 2021

99% of the code is copied from GHCi.UI.info and the functions called from there. Ideally this would be exported from GHCi.UI but sadly that's not the case.

maybe it worths open a ticket in ghc asking for make it public?

@akrmn i was about to open an issue in the ghc project but is not obvious for me what definitions from GHCi.UI.Info (at least gazing at code of master version: https://gitlab.haskell.org/ghc/ghc/-/blob/master/ghc/GHCi/UI/Info.hs) could be exported to avoid the duplication in our hanlding here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Label to trigger pull request merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for displaying :info
3 participants