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

Modes for displaying export lenses #158

Open
wclr opened this issue Nov 24, 2021 · 11 comments
Open

Modes for displaying export lenses #158

wclr opened this issue Nov 24, 2021 · 11 comments

Comments

@wclr
Copy link
Contributor

wclr commented Nov 24, 2021

Looking at new features, suggestions for displaying export lenses.

  1. Remove function names from the display, I don't see how those names are of use here, but they pollute the space, so it could be:

exported (remove myCoolFunction from exports)
->
exported (remove from exports)

private (add myCoolFunction to exports)
->
private (add to exports)

exported (export only myCoolFunction) | (export evertything but myCoolFunction)
->
exported (export only this) | (export evertything else)

  1. To have a short mode which will display only exported/private one-word notice - just to keep things even cleaner and less distrustful. For implicitly exported things I would propose to leave the notice the same: exported (export only this) | (export everything else) because it is usually no good to have everything implicitly exported.

What do you think?

@i-am-the-slime

@nwolverson
Copy link
Owner

Agree on the removal of superfluous names. Not convinced on the addition of additional config just to change the wording of something, feels like excessive config at this point rather than say settling on a good middle ground in terms of wording.

@wclr
Copy link
Contributor Author

wclr commented Nov 24, 2021

I probably would prefer this:

image

to this:

image

Because I don't like unnecessary repeating noise in the code meaning actually nothing, and in this case proposing me to take some action that probably I not going to do in most cases when I look at it (I mean: remove from exports or add to exports), but the notice of exported/private can be useful anyway.

feels like excessive config

Well, this config costs really little and makes things more convenient, at least for some users. We may ask people what they think.

@nwolverson
Copy link
Owner

I initially feel like I prefer the second, because it is not clear to me what the random "exported" words lying around mean, or what clicking on them does - but I think the style of text should maybe be influenced by the conventions around code lenses.

After some reflection the form of the word "exported" makes it clear that it is a noun and not a verb, the state is clear and hence the toggle action - which can often be a UX problem when displaying toggle state, is this describing the current state or what is going to happen.

Reading a long-ago post collecting code lens examples from vscode: https://code.visualstudio.com/blogs/2017/02/12/code-lens-roundup

I'm happy to leave this open for a while and canvas opinion.

@wclr
Copy link
Contributor Author

wclr commented Nov 24, 2021

Found a setting for changing color for codeLens:

"editorCodeLens.foreground": "#666"

So it can be made less distractive:

image

But I think I still would prefer a shorter version (because exessive explicitness here doesn't give anything, and it is repeated too often).

After some reflection the form of the word "exported" makes it clear that it is a noun and not a verb, the state is clear and hence the toggle action - which can often be a UX problem when displaying toggle state, is this describing the current state or what is going to happen.

Besides this can be considered a kind of advanced case (for users that know what it means).

private could be replaced with not exported (though I like the word private here, it just reflects state, without any negation, though need to think...)

image

@wclr
Copy link
Contributor Author

wclr commented Nov 24, 2021

So with short notice, we just give a user an indication of the state.

But when we actually want to give a hint that probably something wrong here, you may need to take some action, we are more verbose and explicit anyway:

image

Also quite important: a long message increases the chance that it will be clicked unintentionally.

@i-am-the-slime
Copy link
Contributor

I think it should be possible to toggle the lenses at the top of the file near the module definition. Managing exports is something that probably doesn't need to be done all the time but at specific times.

Then I think the lenses should have the long names, so it's clear that they're clickable. Otherwise we lack a signifier for the action.

@wclr
Copy link
Contributor Author

wclr commented Nov 26, 2021

I think it should be possible to toggle the lenses at the top of the file near the module definition.

Could you elaborate on what actions should be available near the module definition?

Then I think the lenses should have long names, so it's clear that they're clickable.

As for lenses above functions, as I noticed above, long names not only bring more distraction, but also increase the chance that they will be clicked unintentionally, and this could be a source of annoyance and even mistakes.

@nwolverson
Copy link
Owner

Some updates in latest, moving to the "intermediate length" text that doesn't mention the identifier as I think that is at least some kind of starting point. A couple of additional actions/lenses around data constructor stuff, that I missed when grabbing these original changes

@wclr
Copy link
Contributor Author

wclr commented Dec 10, 2021

Just for notice, this is how Elm's lenses look like:

image

@i-am-the-slime
Copy link
Contributor

@wclr what happens when you click on them?

@wclr
Copy link
Contributor Author

wclr commented Jun 14, 2022

@wclr what happens when you click on them?

Unexpose/expose action.

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

3 participants