[WIP]Type Inference in hint list. #11949

Merged
merged 14 commits into from Dec 10, 2015

Projects

None yet

8 participants

@swmitra
Contributor
swmitra commented Nov 23, 2015

Show type data in the hint list. This is work in progress as we need more ideas about the type symbols which are just text in this PR.
types

A snapshot of the rendered types from this implementation.

@swmitra
Contributor
swmitra commented Nov 23, 2015
@swmitra swmitra added this to the Release 1.6 milestone Nov 23, 2015
@swmitra swmitra self-assigned this Nov 23, 2015
@sprintr
Contributor
sprintr commented Nov 23, 2015

I believe it would be nice to show the data types as the output of typeof(instance) such as "string", "number", "function".

Edit: We can also use "keyword" to show type of keywords.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures

@swmitra
Contributor
swmitra commented Nov 23, 2015

Thanks @sprintr. I will try that once and post a snapshot of the rendering. Our goal is to have the type data without being too loud. We have to compare different displays in order to conclude.

@swmitra
Contributor
swmitra commented Nov 24, 2015

@sprintr With some CSS changes and new literals , this is how it looks.

types1

I must confess , to me it looks cluttered.

@petetnt
Member
petetnt commented Nov 24, 2015

@swmitra maybe aligning the literal labels to right would help (plus maybe more padding to the right side). I think that the labels the OP look pretty nice too, but then again the literals are more clearer (in a way). Maybe the labels could come from a labelProvider for a customization option 🍻

What about mixed types? For example [0, "string", {object: "foo"}]

@abose
Contributor
abose commented Nov 24, 2015

+1 for right align.
+1 for short symbols as in the initial implementaion. Maybe a hybrid- on hover/arrow key on current selected entity, we could expand the short symbol to detailed string?

@ficristo
Member

FWIW, I like the second screenshot with right alignment.
I looked a bit in VisualStudioCode which has the inference type too: I like it can inference the return type of a function too.

@swmitra
Contributor
swmitra commented Nov 24, 2015

@petetnt @ficristo @abose @sprintr A snap with right aligned symbols and more padding.
types2

@swmitra
Contributor
swmitra commented Nov 24, 2015

@petetnt I will write a label provider which can be overridden by extensions.
autocomplete plus works in the same way I guess.
Also I have to infer a mixed type array , will update the PR with the check.

@ficristo
Member

@swmitra Can you try to put the type on the right of the variable names?

@abose
Contributor
abose commented Nov 24, 2015

😅 I too meant right align after the variable names!

@sprintr
Contributor
sprintr commented Nov 24, 2015

This looks good. I am not sure if short names will look good in right alignment.
+1 for right alignment.

@swmitra
Contributor
swmitra commented Nov 24, 2015

@sprintr @petetnt @ficristo @nethip Was trying with different styles. This one
types3
stands out with a small css tweak.

@swmitra
Contributor
swmitra commented Nov 24, 2015

@larz0 @ryanstewart Please provide your inputs.

@swmitra
Contributor
swmitra commented Nov 24, 2015

Some more tuning with the type details ( font-style and weight change )
types4

@ficristo
Member

Just throwing a couple of ideas:

  • the icon and the type should gray and only the one selected ones should be blue so the focus is on the autocomplete (or simply found some way to make the autocomplete a bit more outstanding)
  • the icons seem ot use another style but they are interesting, maybe make them use less space
  • I don't like much the italic version but the word keyword look better there. In the previous screen it has a too much different style

@swmitra great job 👍

@swmitra
Contributor
swmitra commented Nov 24, 2015

Thanks @ficristo . Something like this -
types5

@ficristo
Member

As usual my english need some improvements...
I like the smaller icons but I meant that the left padding should equal to the right: so reduce the right padding. Then the icon will be centered and it will use less space.
The keywords should remain in blue so they stand out more.

And then I stop here.
@swmitra Thank you for your the effort.

@sprintr
Contributor
sprintr commented Nov 24, 2015

I think the function argument list can be omitted from the code hints (see var3 in the above screenshot) since brackets already supports it. This will make code hints small and tidy.

2015-11-24 19_40_48- main js html - brackets

@swmitra
Contributor
swmitra commented Nov 24, 2015

Thanks a lot @sprintr for your prompt feedback. I agree that it doesn't exactly look tidy , but from a developer point of view , I think this adds more flexibility even before I choose the guess. In it's current form, the hinting module allows the parameters hinting only after choosing the guess from the list. But in the approach above we get to see the param type information upfront.

I know it's debatable , but that's why communities are best. In a day the entire ux got changed and became considerably more informative.

@swmitra
Contributor
swmitra commented Nov 25, 2015

Snapshots under default dark theme and light theme.
types6dark
types6

@sprintr
Contributor
sprintr commented Nov 25, 2015

@swmitra Maybe we could take a leaf out of pref code hints where lengthy descriptions are shown under the hint. The type could come on the right and the function signature could come under the hint.

@swmitra
Contributor
swmitra commented Nov 25, 2015

@sprintr I was actually planning to use that for available docs for the particular definition when highlighted.

@abose
Contributor
abose commented Nov 25, 2015

@swmitra Docs in hints is brilliant!
Don't know about the space utilization though, it would mean that other variables might be pushed down which would call for a not so helpful code hints. a mock Screenshot would help.

@Denisov21
Contributor

👍 for short symbols as in the initial implementaion
Wonderful project!

@abose
Contributor
abose commented Dec 8, 2015

Tagging @larz0 for ui review.

@swmitra
Contributor
swmitra commented Dec 8, 2015

Uploading new screenshots for brackets light and dark theme.
latest_light
latest_dark

'<>' in Red indicates an external link for more details. Fetched from JS doc.

@swmitra
Contributor
swmitra commented Dec 9, 2015
@sprintr sprintr commented on the diff Dec 9, 2015
...ault/JavaScriptCodeHints/styles/brackets-js-hints.css
+ color: #d4d4d4;
+ word-wrap: break-word;
+ white-space: normal;
+ width:400px;
+ box-sizing: border-box;
+}
+
+.dark .jshint-description {
+ color: #696969;
+}
+
+.jshint-jsdoc {
+ display: none;
+ padding-left: 28px !important;
+ padding-right: 10px !important;
+ color: grey;
@sprintr
sprintr Dec 9, 2015 Contributor

It doesn't look very visible in the dark mode. We use #ccc in pref hints so it will be good to use it here as well.

Dark Mode: #cccccc
Light Mode: #6e6e64

@sprintr
Contributor
sprintr commented Dec 9, 2015

Hints with lengthier signature are breaking the layout of individual hints.

2015-12-09 16_00_14- main js html - brackets
2015-12-09 16_08_51- main js html - brackets

@swmitra
Contributor
swmitra commented Dec 9, 2015

@sprintr It is intentional to make the type details appear in next line if it is beyond a certain character length.

@abose
Contributor
abose commented Dec 9, 2015

It will look better if the signature is also right aligned in the second line.

@sprintr
Contributor
sprintr commented Dec 9, 2015

Maybe we could remove the 5px padding on the signature then.

@sprintr
Contributor
sprintr commented Dec 9, 2015
// 1
function Get() {
    var h1 = document.
}
// 2
function Get() {
    var h1 = document.crea
}

Brackets now throws an error and doesn't give any code hints for script 1 (from dot up to 3 chars) whereas script 2 works just fine.

@swmitra
Contributor
swmitra commented Dec 9, 2015

@sprintr Fixed now. jQuery was complaining as it was thinking that js doc spans innerHtml was malformed ( as it contains html tags sometimes ). Fixed that by adding the doc as text instead of html.

@sprintr
Contributor
sprintr commented Dec 9, 2015

@swmitra Great. works fine now.

@swmitra
Contributor
swmitra commented Dec 10, 2015

@sprintr Replaced '<>' with a boxed '?'. Looked better to me. Please have a look and let me know.

@sprintr
Contributor
sprintr commented Dec 10, 2015

@swmitra Looks better to me too. <> had an html like feeling to it.

@abose abose merged commit 60eaca4 into master Dec 10, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@abose abose deleted the swmitra/TypeInferenceInHintlist branch Dec 10, 2015
@abose
Contributor
abose commented Dec 10, 2015

Thanks @swmitra + @petetnt @ficristo @sprintr @Denisov21 For this feature 🎆
All unit tests/ extension tests for code hints passing.
Tagging @larz0 for UI Review.
Please log issues if any found for the feature.

Merging to give better testing window before shipping.

@nethip
Contributor
nethip commented Dec 10, 2015

@swmitra Great work! This is awesome.

Small recommendation. I think the hints list looks cleaner without the box icons on the left. Also we could do something better with replacing current icon "?". Let us go without the box icons for now. We could then incorporate the changes(or icons) once we hear from @larz0 .

@ex1st
ex1st commented Jan 18, 2016

Map and Set have been forgotten.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment