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

Doc export file paths all start with ile:// #2971

Open
rhys-vdw opened this issue Nov 28, 2024 · 7 comments
Open

Doc export file paths all start with ile:// #2971

rhys-vdw opened this issue Nov 28, 2024 · 7 comments

Comments

@rhys-vdw
Copy link

How are you using the lua-language-server?

Command Line

Which OS are you using?

Windows WSL

What is the issue affecting?

Other

Expected Behaviour

Paths in exported doc JSON should start with file:/// as in the example:

"file": "file:///c%3A/Users/Me/Documents/Array.lua",

Actual Behaviour

Missing the leading f:

"file": "ile:///home/rhys/spring/./cont/base/springcontent/LuaHandler/handler.lua",

Reproduction steps

rhys@RhysDesktop:~/spring$ lua-language-server --doc . --doc_out_path docs/
root uri = file:///home/rhys/spring/.
Documentation exported:91/2488

docs/doc.json
docs/doc.md

Additional Notes

rhys@RhysDesktop:~/spring$ lua-language-server --version
<Unknown>
rhys@RhysDesktop:~/spring$ lua-language-server --help
Content-Length: 55

{"jsonrpc":"2.0","method":"$/hello","params":["world"]}

How do you get a version?

Log File

No response

@rhys-vdw
Copy link
Author

Ah...

rhys@RhysDesktop:~/spring$ brew info lua-language-server
==> lua-language-server: stable 3.13.0 (bottled), HEAD

@tomlau10
Copy link
Contributor

This may be a related issue: #2964, which points out that the file uri format seems changed since a recent refactor PR #2821 by a contributor.

I strongly suspect that there maybe some hidden bugs in that refactor PR. 🤔

@rhys-vdw
Copy link
Author

We are currently working on a full documentation export for a large RTS engine, so it should be a good test case. Is there a prior release version we can try rolling back to to compare doc export before this PR was merged?

@tomlau10
Copy link
Contributor

By looking at the release page, #2821 is released in v3.10.6, so I guess you may try the prior release v3.10.5 🤔

@rhys-vdw
Copy link
Author

I can't work out how to install a historic version with brew.

rhys@RhysDesktop:~/spring/doc/site$ brew install [email protected]
Warning: No available formula with the name "[email protected]". Did you mean lua-language-server?
==> Searching for similarly named formulae...
==> Formulae
lua-language-server

To install lua-language-server, run:
  brew install lua-language-server
rhys@RhysDesktop:~/spring/doc/site$ brew search lua-language-server
==> Formulae
lua-language-server                    vala-language-server                   yaml-language-server                   sql-language-server                    bash-language-server

Is this possible?

@rhys-vdw
Copy link
Author

Okay, I just installed directly from the GitHub releases. I can confirm that this version does not have the above issue, and also fixes #2977. It seems this PR may have introduced a number of issues.

@tomlau10
Copy link
Contributor

I just took some time to debug into this, found the logic that causing this issue, but have yet come up with a good solution 😕

  • the file path of each file is transformed by this export.getLocalPath()
    function export.getLocalPath(uri)
    --remove uri root (and prefix)
    local local_file_uri = uri
    local i, j = local_file_uri:find(DOC)
    if not j then
    return '[FORIEGN]'..uri
    end
    return local_file_uri:sub( j + 1 )
    end
  • the uri here is the file:///... string
  • the DOC global here is the --doc=xxx value that passed in via CLI

I believe this logic intends to remove the path prefix, such that each paths in the output are relative to the supplied directory. For example:

  • /data/test/a.lua
  • /data/test/b.lua
  • by using --doc=/data/test
    => the expected paths in output md/json will just be /a.lua, /b.lua

‼️ However when using --doc=., since . means match anything in a standard string.find()
=> this just match the 1st char
=> j = 1 (the end index of the match)
=> so the last statement return local_file_uri:sub( j + 1 ) gives ile://... 🤦‍♂️

Now you may think that we can simply pass plaintext=true in string.find
=> local_file_uri:find(DOC, 1, true)
But that's not enough ‼️
Because DOC can just be ., and there can be cases that the project folder is located in some directory with name aaa.bbb
=> this will still matches the . in the middle


Workaround 1

  • use absolute path when invoking from cli, such as
lua-language-server --doc=$PWD --doc_out_path docs/

=> each file path will now be /a.lua, /b.lua, ...

  • if you don't want to / at the begining, then use $PWD/
lua-language-server --doc=$PWD/ --doc_out_path docs/

=> each file path will now be a.lua, b.lua, ...

Workaround 2

Judging from the diff in #2821, the original handling of file name is just to use guide.getUri()
And that #2821 actually provides a Lua.docScriptPath config for you to override the functions in that export module.
So you can have the following:

  • in .luarc.json
   ...
   "docScriptPath": "export.lua"
  • export.lua
function export.getLocalPath(uri)
    return uri
end
  • finally trigger through cli using absolute path with /
lua-language-server --doc=$PWD/ --doc_out_path docs/

=> now you should have the classic uri result

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants