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

Functions in struct in "module : sig ... end = struct ... end" not shown in Structure panel #170

Closed
jfehrle opened this issue Apr 27, 2019 · 6 comments

Comments

@jfehrle
Copy link
Collaborator

jfehrle commented Apr 27, 2019

plugin version: 0.76.2

Description

  1. The structure panel shows the 4 methods listed in the signature for Backtrack but the other functions defined in the struct such as fold_until don't appear anywhere in the Structure panel. Seems like they should be shown somewhere.

image

  1. Double clicking on any of the Backtrack methods takes you to the signature rather than the function body. Probably would be more helpful to jump to the function body.

The code is in https://github.com/coq/coq/blob/master/stm/stm.ml

@giraud
Copy link
Owner

giraud commented Aug 22, 2019

in master, impl is shown if there is both signature+body:

impl2

Implementation is always at the bottom
It removes the need for 2. , you can click on signature/body method

@jfehrle
Copy link
Collaborator Author

jfehrle commented Aug 22, 2019

Looks good! Maybe put "Implementation" in parentheses so it can't be mistaken for the name of a submodule. The icon suggests that it's a module, but I don't have a good suggestion on how to do it better.

I suppose you'll release this soon?

@giraud
Copy link
Owner

giraud commented Aug 24, 2019

Yes, I'm planning a release soon.

many options are possible:
a
impl_a

b
impl_b

c
impl_c

d
impl_d

e
impl_e

@jfehrle
Copy link
Collaborator Author

jfehrle commented Aug 25, 2019

"Backtrack (implementation)" is too long, so I'd eliminate e.

I would omit the icon next to the implementation label because it's not a module, so I'd eliminate a and d. In c, Backtrack.impl might be taken as related to a submodule impl at first glance. I think I'd go for b, but maybe use just (impl) as the label.

A couple other thoughts:

  • It would be nice if the items under "implementation" lined up with those under the module name since they're really at the same level. But the tree control probably doesn't support this.
  • I tend to think of the implementations as being public (visible outside) or private (not visible), akin to Java. Might be nice to indicate this somehow if it's easy.
  • The V icon is used both for values (non function let statement) and entries (val ...) in the module signature. I tend to think of the vals as another form of type info. Perhaps they should be shown with the T icon?

image

giraud added a commit that referenced this issue Aug 25, 2019
@giraud
Copy link
Owner

giraud commented Aug 28, 2019

the (impl) is present in 0.79.

@jfehrle
Copy link
Collaborator Author

jfehrle commented Aug 29, 2019

Looks very good, thanks!

@giraud giraud closed this as completed Oct 18, 2019
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

No branches or pull requests

2 participants