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.

Sign up for GitHub

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

Jump to bottom

Fixed the error that the configuration file pointed to by the --configpath option was not read and loaded. #3012

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fixed the error that the configuration file pointed to by the --configpath option was not read and loaded. #3012

wants to merge 4 commits into from
+23 −11

Conversation

Mc-GrowlR
Copy link

@Mc-GrowlR Mc-GrowlR commented Dec 27, 2024 •
edited
Loading

fixes #3008, fixes #2997

Info

Follow-up on my issue #3008 .Screenshots of some of the effects can also be found in the issue.
This commit is intended to fix a bug where exporting a document via the command line could not read the configuration file specified with the '--configpath' option.

ENV

.luarc.json file

from: #2997
.luarc.doc.json

command

./bin/lua-language-server.exe --configpath ./.luarc.doc.json --doc ./test/main.lua --doc_out_path ./test/

test lua file

M = {}

result

output json doc file:

doc.json

Add the terminal output of the print statement

root uri        file:///e%3A/luals/old/test/main.lua
Config All Path:        E:/luals/old/.luarc.doc.json
configName:     .luarc.doc.json
parent: E:/luals/old
------
uri type:       string  file:///e%3A/luals/old
CONFIGPATH      ./.luarc.doc.json
aaa     .luarc.doc.json

==================
loadLocalConfig Test Begins
folderPath      E:\luals\old
after :         E:\luals\old\.luarc.doc.json
loadLocalConfig Test end
 ================

path:   E:\luals\old\.luarc.doc.json
 buf    string  {
  "$schema": "https://raw.githubusercontent.com/sumneko/vscode-lua/master/setting/schema.json",
  "runtime.version": "Lua 5.1",
  "runtime.builtin": {
    "basic": "disable",
    "bit": "disable",
    "bit32": "disable",
    "builtin": "disable",
    "coroutine": "disable",
    "debug": "disable",
    "ffi": "disable",
    "io": "disable",
    "jit": "disable",
    "math": "disable",
    "os": "disable",
    "package": "disable",
    "string": "disable",
    "table": "disable",
    "table.clear": "disable",
    "table.new": "disable",
    "utf8": "disable"
  }
}
Documentation exported:

test\doc.json
test\doc.md

Postscript

I believe that with the fix of this bug, you can use the document export function, no need to make changes to the source code, and only need the configuration file to filter out the standard libraries in the environment.
If I can help you, then my work is meaningful.

@Mc-GrowlR
Copy link
Author

fix a lot of bug

@Mc-GrowlR
Copy link
Author

I tried to use as few changes as possible to make sure that the behavior when no profile was specified was the same as before.
One more thing, when changing one of the files, I forgot to close the code formatting plugin, I am sorry for the changes in it, I only found out after the merge. This is my first time submitting a PR, please give me advice if there is anything wrong.

tomlau10
tomlau10 reviewed Dec 28, 2024
View reviewed changes
Copy link
Contributor

@tomlau10 tomlau10 left a comment

Choose a reason for hiding this comment

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

I think that using runtime.builtin disable is not the best way to filter out the standard libraries in the environment.
Something should be done in export.gatherGlobal as said in this comment: #2997 (comment)
Maybe a new option to control that would be even better. 😕

Anyway this PR aims to fix the --configpath issue so it is no worse. 👍
And if you believe it can close your original issue, you should include this in the first line of your PR description (just edit it is enough):

fixes #3008, fixes #2997

=> this let github correctly links this PR to close the corresponding issue on merge 😄
https://docs.github.com/en/issues/tracking-your-work-with-issues/using-issues/linking-a-pull-request-to-an-issue

script/cli/doc/init.lua Outdated Show resolved Hide resolved
script/provider/provider.lua Outdated Show resolved Hide resolved
script/cli/doc/init.lua Outdated Show resolved Hide resolved
@tomlau10
Copy link
Contributor

I don't think this PR will fix #2977 on its own 😕 .
This just fixes the issue that when executing a --doc command, the --configpath option is ignored.

For general users when they do a --doc without any special/easy settings, still all built-in libraries will still be outputted to the json/md.
Thus the --configpath method is at most a workaround only, but not a fix?

@Mc-GrowlR
Copy link
Author

Mc-GrowlR commented Dec 28, 2024 •
edited
Loading

I don't think this PR will fix #2977 on its own 😕 .我认为这个 PR 不会自行😕修复 #2977 。 This just fixes the issue that when executing a --doc command, the --configpath option is ignored.这只是解决了在执行 --doc 命令时忽略该 --configpath 选项的问题。

For general users when they do a --doc without any special/easy settings, still all built-in libraries will still be outputted to the json/md.对于普通用户来说，当他们做一个 --doc 没有任何特殊/简单的设置时，所有内置库仍然会输出到 json/md 中。 Thus the --configpath method is at most a workaround only, but not a fix?因此，该方法 --configpath 最多只是一个解决方法，而不是修复方法？

Okay, I'll remove the change issue.
You're right, it's a different issue about the handling of these standard libraries, and this PR just fixes a bug in one of the ways to solve the problem.

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

@tomlau10 tomlau10 tomlau10 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Regarding the resolution of the '--configpath' issue. Documentation generation does not respect --configpath
2 participants
@Mc-GrowlR @tomlau10