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.

Sign up for GitHub

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

improvement:don't show types for match case for showInferredType #5284

Merged
merged 9 commits into from
Jul 21, 2023
Merged

improvement:don't show types for match case for showInferredType #5284

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
cc5bab4
improvement:don't show types for match case when showInferredType
doofin May 28, 2023
08b1a47
improvement: more granularity for showInferredType option
doofin Jun 6, 2023
b382d70
improvement:don't show types for match case when showInferredType
doofin May 28, 2023
e2ab166
improvement: more granularity for showInferredType option
doofin Jun 6, 2023
0a87952
Merge branch 'simpleTypeHint' of github.com:doofin/metals into simple…
doofin Jun 6, 2023
54a33aa
improvement: fix test for showInferredType option
doofin Jun 6, 2023
4054d79
improvement: finally fix test for showInferredType and backward compa…
doofin Jun 7, 2023
7e75641
Update metals/src/main/scala/scala/meta/internal/decorations/Syntheti…
doofin Jul 1, 2023
32f3878
improvement: add test for showInferredType
doofin Jul 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion metals/src/main/scala/scala/meta/internal/decorations/SyntheticsDecorationProvider.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,8 @@ final class SyntheticsDecorationProvider(
case param: m.Term.Param =>
if (param.decltpe.isEmpty) List(param.name.pos.toSemanticdb) else Nil
case cs: m.Case =>
explorePatterns(List(cs.pat)) ++ visit(cs.body)
// if the case is too long then it'll be too messy
visit(cs.body) // explorePatterns(List(cs.pat)) ++ visit(cs.body)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't agree about removing this, since it might be useful to know the type in case.

We might be over eager in case of inferred type for methods, we could for sure separate it under another flag (or rethink the settings here to be more useful).

Any other opininos?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It could also be nice to provide more options like showInferredType = minimal| full | etc...

Copy link
Member

Choose a reason for hiding this comment

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

+1 for more granularity

case vl: m.Defn.Val =>
val values =
if (vl.decltpe.isEmpty) explorePatterns(vl.pats) else Nil
Expand Down