angelozerr / tern.java

Use tern.js in Java context
http://ternjs.net/
Other
249 stars 52 forks source link

Generate inline function when completion apply is done #134

Closed angelozerr closed 9 years ago

angelozerr commented 9 years ago

When function signature as a parameter which is a function like express app.get :

var express = require('express');
var app = express();

app.get // Ctrl+Space

When completion app.get is applyed it generates :

var express = require('express');
var app = express();

app.get(path, callback);

It should be better that it's generate the inline function :

var express = require('express');
var app = express();

app.get(path function(req, res){

});
ghost commented 9 years ago

@angelozerr: beside the missing coma in your "better" code completion, I totally aggree with you: a callback should be inlined as function().

Does it require a modification in tern-node-express json data or in tern implementation?

What would happen if the function was named "function" instead of "callback"?

Its params won't show either I suppose...

angelozerr commented 9 years ago

I totally aggree with you: a callback should be inlined as function().

Glad you like this idea:)

Does it require a modification in tern-node-express json data or in tern implementation?

To do that I must modify Eclipse Java tern completion apply https://github.com/angelozerr/tern.java/blob/138c6040188491791a9c2a09df7ffcd741098173/eclipse/tern.eclipse.ide.ui/src/tern/eclipse/ide/ui/contentassist/JSTernCompletionProposal.java#L167

The apply method should use information about fn callback of JSON Type Definition like

get {
fn(path: string, callback: [fn(req: +request.Request, req: +response.Response)]) -> !this
}

The Java pply method will generate :

get(path, function(req, res) {

})

instead of :

get(path, callback)

What would happen if the function was named "function" instead of "callback"?

You mean parameter name like :

get {
  "!type" : "fn(path: string, function: [fn(req: +request.Request, req: +response.Response)]) -> !this"

If it that, it generates today function like this :

get(path, function)
angelozerr commented 9 years ago

@gamerson I think this feature could be interesting too for AUI.

ghost commented 9 years ago

@angelozerr

get: { fn(path: string, callback: [fn(req: +request.Request, req: +response.Response)]) -> !this }

As far I understand it we have an array of callback functions in your example above.

What about this auto-completion pattern in such a case:

app.get(path, function(req, res), ... ) { }

angelozerr commented 9 years ago

I though generate one function.

Perhaps we should have too some preferences like :

Those preferences could be customized in several level :

and be customized per modules too.

I don't know if it's very interesting to do that?

ghost commented 9 years ago

I don't know if it's very interesting to do that?

The usual case with express is one single inline callback function – passing it several might be quite confusing actually.

So - if we go with one single callback then we might modify the json definitions in tern-node-express to remove the arrays in callbacks.

When a nodejs developer is passing several callbacks we'll assume she knows what she is doing therefore auto-completion is less necessary – is this making sense?

Otherwise we should support the array in auto–completion and in such a case the ellipsis is fine to me.

ghost commented 9 years ago

When tern autocompletion simplifies an API signature the actual API might be given at the top of its '!doc' section using a bold typeface on a first line maybe?

gamerson commented 9 years ago

Yes I think so since AUI has so many function(A){...} callbacks.

On Mon, Oct 6, 2014 at 4:50 PM, Angelo notifications@github.com wrote:

@gamerson https://github.com/gamerson I think this feature could be interesting too for AUI.

— Reply to this email directly or view it on GitHub https://github.com/angelozerr/tern.java/issues/134#issuecomment-57989603 .

Greg Amerson Liferay Developer Tools Liferay, Inc. www.liferay.com

angelozerr commented 9 years ago

See https://github.com/angelozerr/tern.java/wiki/Tern-Advanced-Completion

paulvi commented 9 years ago

It should be better that it's generate the inline function :

While it may be good for express, generally too may inlines create Callback Hell

I just wonder what UX should be....

What if user gets app.get(path, callback); but than can use Ctrl+1 to replace undefined callback to actually inline function or define it elsewhere. (Eclipse JDT has several nice options for undefined variables, that I personally prefer over manually defining variables)

P.S. I just read wiki, at it seems that Angelo has already did it

Since 0.7.0, Tern IDE provides the "generate anonymous function" option

angelozerr commented 9 years ago

While it may be good for express, generally too may inlines create Callback Hell

Oj if I understand you wish to have preferences to activate "generate anonymous function" for some modules (just for express for instance). I don't know if it's possible to do that. If it's not possible, I hav eth eintention to add a global option for that (anabled/disabled the "generate anonymous function" mode).

What if user gets app.get(path, callback); but than can use Ctrl+1 to replace undefined callback to actually inline function

it's a cool idea, but I'm afraid that it will hard to do (I'm afraid that tern is not able to tell that callback belongs to app.get function

P.S. I just read wiki, at it seems that Angelo has already did it

Yes "generate anonymous function" is available for 0.7.0 (you can install 0.7.0-SNAPSHOT to test it). I must manage know the enable/disable option.

paulvi commented 9 years ago

if I understand you wish to have preferences to activate "generate anonymous function" for some modules

I have not wished. It may be considered in the future, but for now global option is just nice.

What if user gets app.get(path, callback); but than can use Ctrl+1 to replace undefined callback to actually inline function

it's a cool idea, but I'm afraid that it will hard to do (I'm afraid that tern is not able to tell that callback belongs to app.get function

I though more primitive to handle word "callback". If apply tern.js, it can tell that this variable is undefined and should be function, so TernIDE could propose that.

For 0.7.0 global enable/disable option would be enough.

angelozerr commented 9 years ago

If apply tern.js, it can tell that this variable is undefined and should be function, so TernIDE could propose that.

Perhaps we could do that with tern lint. I would like to support quick fix https://github.com/angelozerr/tern-lint/issues/24

angelozerr commented 9 years ago

Ok this issue is finished, now you can enable/disable the "generate anonymous function". See https://github.com/angelozerr/tern.java/wiki/Tern-Advanced-Completion to see preferences screenshot

paulvi commented 9 years ago

Once again why it is project settings, but not workspace setting ?

paulvi commented 9 years ago

Once again why it is project settings, but not workspace setting ?

OK, I see now it should be in Workspace as well.