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

[display] @:deprecated typedefs in completion #8178

Closed
Gama11 opened this issue Apr 17, 2019 · 14 comments · Fixed by #8207
Closed

[display] @:deprecated typedefs in completion #8178

Gama11 opened this issue Apr 17, 2019 · 14 comments · Fixed by #8207
Assignees
Labels
enhancement feature-ide IDE / Editor support
Milestone

Comments

@Gama11
Copy link
Member

Gama11 commented Apr 17, 2019

It's rather annoying to have the now deprecated types (#7390) show up in completion, and before the ones you're meant to use:

I think there's no reason to show @:deprecated typedefs in completion at all? At least as long as they're just simple aliases, typedefs to anon structs should probably still be shown. I think there's already some detection for the difference for nicer auto-generated deprecation messages.

@Gama11 Gama11 added enhancement feature-ide IDE / Editor support labels Apr 17, 2019
@Gama11 Gama11 added this to the Release 4.0 milestone Apr 17, 2019
@EricBishton
Copy link
Member

Filter this in vsHaxe or vsCode, not the compiler. IDEs should be able to make their own decisions.

@Gama11
Copy link
Member Author

Gama11 commented Apr 17, 2019

I guess that might be preferable. But in either case something needs to change in the compiler, because the protocol currently doesn't seem to include metadata for types (even though the data structure has a meta field).

It looks like metas are only sent during resolve right now, and they're rather heavy since they include a pos for some reason..

@EricBishton
Copy link
Member

I agree the metadata should be included in the protocol response. A flag set for common metadata would be Ok, if we want something lightweight. (Still need all of the metadata to be present, though.)

@Gama11
Copy link
Member Author

Gama11 commented Apr 17, 2019

The thing I'm concerned about is this pos stuff, which seems rather useless in this context:

We probably don't even need that during resolve requests.

@EricBishton
Copy link
Member

We could use a file dictionary to remove the duplicate — very long — names.

@Gama11
Copy link
Member Author

Gama11 commented Apr 17, 2019

I don't think we need the positions of the metadata at all. There's already a position on the type.

@EricBishton
Copy link
Member

That may be true for this use case. The question becomes whether the code that puts out the metadata is common and the position is required for (an)other use case(s). For instance, Is this code part of the compiler trying to expose full fidelity parsed data?

Or, is it, perhaps, trying to give ordering of the metadata so that an IDE can re-order based upon the user's/IDE's rules?

@Simn
Copy link
Member

Simn commented Apr 18, 2019

Never thought I'd see the day where @Gama11 wants less positions in the display output...

@RealyUniqueName
Copy link
Member

What about an opt-in argument for display requests to add meta positions. By default, metadata won't contain pos infos.

@Gama11
Copy link
Member Author

Gama11 commented Apr 18, 2019

As I said, I don't think there's any point to including these positions at all, so a flag seems like overengineering. A list of metas to include in responses might make sense though, to reduce bandwidth if we only look at e.g. :deprecated anyway.

@ncannasse
Copy link
Member

I agree @:deprecated should imply @:noCompletion, on both types and fields. No flag required.

@RealyUniqueName
Copy link
Member

I agree @:deprecated should imply @:noCompletion, on both types and fields. No flag required.

That means the decision about hiding it from completion would be made by the compiler. While the original issue (and I agree with it) suggested to let IDE decide whether a type should be listed in completion.

@Gama11
Copy link
Member Author

Gama11 commented Apr 24, 2019

Well, originally I suggested hiding it compiler-side, but then @EricBishton complained. :) It would be pretty consistent with what's done with @:noCompletion already though (the compiler makes the decision there, as it should so that lib authors can rely on it / some editor doesn't decide to show it anyway).

@RealyUniqueName
Copy link
Member

I have no strong opinion on that. It just feels right to make this decision on IDE side.
@:noCompletion is a different story - it denotes a user explicitly wants to hide something from completion, while @:deprecated is not about completion at all. If that was a case one could manually add @:noCompletion.

Gama11 added a commit to vshaxe/haxe-language-server that referenced this issue Apr 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement feature-ide IDE / Editor support
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants