Skip to content

update #554

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

Open
wants to merge 51 commits into
base: main
Choose a base branch
from
Open

update #554

wants to merge 51 commits into from

Conversation

xuhuanzy
Copy link
Member

No description provided.

export.func = function()
-- When typing `func` in other files, import suggestions will be shown
end
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what this does? I thought completion already works without this.

Copy link
Member Author

@xuhuanzy xuhuanzy Jun 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to enable automatic import of subfields. We can't directly provide import suggestions for subfields of all exported tables, as this would severely pollute the auto-completion hints in a huge project and lead to very poor performance. This is especially true in game projects where Lua is widely used, as they often have a large number of configuration tables that could cause auto-import to freeze.

Here is an example:

-- test1.lua
---@export
local export = {}
export.func = function()
end
func -- When you type 'func', the auto-completion will suggest whether you want to automatically import 'test1.func'.

image

image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another benefit of this approach is that when an export table is marked with ---@export, we can provide diagnostics for undefined fields.

Additionally, we might later provide a diagnostic for disallowed imports, as ---@export can accept a variable. For example, when it's specified as ---@export namespace, it will only allow imports of that file from the same namespace.

Copy link
Collaborator

@lewis6991 lewis6991 Jun 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly I think I use @class on an exported table for a similar effect: to get undefined field diagnostics.

Does using @export have any other benefits over @class

Additionally, we might later provide a diagnostic for disallowed imports, as ---@export can accept a variable. For example, when it's specified as ---@export namespace, it will only allow imports of that file from the same namespace.

Sorry I don't understand what this means. By import do you mean require?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@class does not provide automatic import of subfields, and these two are not conflicting. @class can also include @export.

@export simply tells the LSP how to handle the export of this variable.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@namespace is a namespace. After using @namespace in a file, all subsequent type definitions will automatically have the prefix declared by @namespace added. It also implies @using.

@using is for referencing a namespace and does not affect type definitions.

As for @export, its current primary purpose is to assist with completion, but in the future, we will implement a simple file visibility control for it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

---@export is globally visible by default.
---@export namespace is only visible within the current namespace.
---@export global is globally visible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now, a simple module import visibility control has been added for him. When importing variables marked with @export, if it is ---@export namespace, it will check whether the current file's namespace matches it. If not, a warning will be issued.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I don't understand what this means. By import do you mean require?

yes

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If @export is not marked, everything remains the same as before. @export is only meant to enhance the module import experience and slightly restrict the current situation where imports can be used freely.

@xuhuanzy xuhuanzy force-pushed the update branch 2 times, most recently from 697f34b to 3ff7636 Compare June 28, 2025 00:19
@xuhuanzy xuhuanzy changed the title impl @export update Jun 28, 2025
@xuhuanzy xuhuanzy linked an issue Jun 28, 2025 that may be closed by this pull request
@xuhuanzy
Copy link
Member Author

xuhuanzy commented Jun 29, 2025

Feature: Index members allow defining an alias, which will be used as the display during completion.

If a comment starts with [xxx], it is considered to add an alias for completion to the index, and whitespace characters are allowed before and after the content inside the [].

---@class Point
---@field [1] number # 	[ x ]
---@field [2] number # [y]
---@field [3] number # [z]
---@field [4] number # If there is any non-whitespace character before it, then it is not defining an alias. [error]


local test = {
    [1] = 1, -- [nameA]
}

image

@clason

This comment was marked as resolved.

@CppCXY
Copy link
Member

CppCXY commented Jun 29, 2025

@xuhuanzy since you are rolling all manner of updates into single large PRs, would you mind fixing the following small inconsistency while you're at it? reportsemmylua_check

[duplicate-require] Advice:
<diagnostic>
Hints: 1

The should be reported as for consistency (and same for other "Advices").[duplicate-require]``Hint:

I remember it should be that the library I used didn't have hints.

@clason

This comment was marked as resolved.

@CppCXY
Copy link
Member

CppCXY commented Jun 29, 2025

Yeah, I looked for it and saw that you used ariadne for the output, which (weirdly) only contains Advice. Options:

  1. open a feature request there (arguing that "Error, Warning, Info, Hint" are standard LSP types that should be supported out of the box);
  2. use a custom kind for Hint (and Info?);
  3. report as Advices: ... for consistency;
  4. do nothing.
  1. Perhaps we can also write one ourselves.

@xuhuanzy
Copy link
Member Author

The [duplicate-require] should be reported as Hint: for consistency (and same for other "Advices").

I haven't used emmylua_check, it should be implemented by cppcxy.

@clason

This comment was marked as resolved.

@xuhuanzy xuhuanzy linked an issue Jun 29, 2025 that may be closed by this pull request
@xuhuanzy xuhuanzy force-pushed the update branch 2 times, most recently from ae50534 to fc86b67 Compare June 29, 2025 13:29
@xuhuanzy
Copy link
Member Author

feature: Index access now displays aliases in the hint.

Can be turned off via hint.index_hint

image

@xuhuanzy
Copy link
Member Author

@lewis6991 @clason @CppCXY Is there a better character than using > as a prefix?

@clason
Copy link

clason commented Jun 29, 2025

Hmm, not sure I'd ever use that feature myself, but maybe just put the alias in parentheses?

point[1 (x)]

Or use a colon (to match the type annotation)?

point[1: x] or point[1 :x] (note the spaces; former looks nicer to me, but latter looks more like a "symbol" in other languages, which might be appropriate?)

You could also treat it like a "field name": point[x: 1] to have a uniform style (not sure it makes sense to distinguish index_hints from other hints)?

@lewis6991
Copy link
Collaborator

: seems better than > to me.

@xuhuanzy
Copy link
Member Author

You could also treat it like a "field name": point[x: 1] to have a uniform style (not sure it makes sense to distinguish index_hints from other hints)?

This is a key name, not a type. I didn't use a colon (:) as I think it could be confusing.

@clason
Copy link

clason commented Jun 29, 2025

Then :x is my suggestion. I think the consistency with other hints is useful, and the different order makes enough of a difference.

@CppCXY
Copy link
Member

CppCXY commented Jun 29, 2025

Yeah, I looked for it and saw that you used ariadne for the output, which (weirdly) only contains Advice. Options:

  1. open a feature request there (arguing that "Error, Warning, Info, Hint" are standard LSP types that should be supported out of the box);
  2. use a custom kind for Hint (and Info?);
  3. report as Advices: ... for consistency;
  4. do nothing.

I have refactor the emmylua_check, you can try the new version. I have removed the ariadne.

@clason

This comment was marked as resolved.

@CppCXY
Copy link
Member

CppCXY commented Jun 29, 2025

Nice, and seems faster, too! Minor suggestion: Use underlines (or -----) instead of carets? Or with the color highlighting, the additional marking is redundant? (Although may be better for accessibility to have both.)

I have removed the carets, but some unicode issue need resovle

@CppCXY
Copy link
Member

CppCXY commented Jun 29, 2025

Nice, and seems faster, too! Minor suggestion: Use underlines (or -----) instead of carets? Or with the color highlighting, the additional marking is redundant? (Although may be better for accessibility to have both.)

I have fixed the unicode issues , and it should basically work now.

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