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

Library resolution does not work for use #3

Closed
Lenbok opened this issue May 7, 2022 · 14 comments
Closed

Library resolution does not work for use #3

Lenbok opened this issue May 7, 2022 · 14 comments

Comments

@Lenbok
Copy link
Contributor

Lenbok commented May 7, 2022

As reported on Reddit, for me this openscad language server does not resolve modules that are contained in libraries, whereas dzhu's language server does find my libraries just fine.

I am running on linux, my libraries are installed in the built-in location $HOME/.local/share/OpenSCAD/libraries/

I do not normally use OPENSCADPATH, however, I also tried setting this and it could still not resolve the modules in libraries.

@Leathong
Copy link
Owner

Leathong commented May 8, 2022

@Lenbok Is it work if included file is in same directory?

@Lenbok
Copy link
Contributor Author

Lenbok commented May 8, 2022

Yes, it does seem to work for finding definitions in files that are in the same directory, or found in subdirectories relative to the current file.

@Leathong
Copy link
Owner

Leathong commented May 8, 2022

Yes, it does seem to work for finding definitions in files that are in the same directory, or found in subdirectories relative to the current file.

So the problem is that the directory of the libraries cannot be found correctly, I will check it.

@Lenbok
Copy link
Contributor Author

Lenbok commented May 8, 2022

BTW, I was trying to see in the code where to put some debugging (not that I am familiar with rust), but the naming of the library location functions are inconsistent with the terminology in the descriptions at https://en.wikibooks.org/wiki/OpenSCAD_User_Manual/Libraries:

  • user_library_location should probably be called built_in_library_location
  • env_library_locations should probably be called user_defined_library_locations

It's a minor thing, but it helps to be consistent when describing bugs etc.

@Leathong
Copy link
Owner

Leathong commented May 8, 2022

BTW, I was trying to see in the code where to put some debugging (not that I am familiar with rust), but the naming of the library location functions are inconsistent with the terminology in the descriptions at https://en.wikibooks.org/wiki/OpenSCAD_User_Manual/Libraries:

  • user_library_location should probably be called built_in_library_location
  • env_library_locations should probably be called user_defined_library_locations

It's a minor thing, but it helps to be consistent when describing bugs etc.

That is a good suggestion, and I don't have a Linux development environment, may not be able to fix this soon, if you can fix it, please submit a PR.

@Leathong
Copy link
Owner

Leathong commented May 8, 2022

@Lenbok I don't know why it doesn't work on Linux, I added the log to print the search paths to std out, you can try again to see if your library path is included.

@Lenbok
Copy link
Contributor Author

Lenbok commented May 8, 2022

I had to change your log_to_console function to send to stderr, since emacs only logs that (I guess stdout is just used for those servers using stdio), but I see it does have my library path:

[server] start sucess
[server] search paths:
[server] file:///home2/len/.local/share/OpenSCAD/libraries/
[server] file:///usr/share/openscad/libraries/

My built-in library folder contains libraries such as:

len@tron:openscad-LSP master(+4/-4)+  ls -l /home2/len/.local/share/OpenSCAD/libraries/
total 84
drwxrwxr-x  2 len relaxo  4096 May 24  2018 bezier
drwxrwxr-x  9 len len     4096 Apr  7 12:17 BOSL2
drwxrwxr-x  2 len relaxo  4096 Mar 10  2018 etc
-rw-r--r--  1 len relaxo  4016 Jan 22  2014 gear_rack.scad
drwxrwxrwx  2 len relaxo  4096 May 20  2017 Knurled_Surface
drwxr-xr-x  3 len relaxo  4096 Nov  6  2021 Lenbok_Utils
drwxrwxr-x  4 len len     4096 Sep 24  2019 list-comprehension-demos
drwxr-xr-x  6 len relaxo  4096 Mar 12  2014 MCAD
drwxrwxr-x  2 len relaxo  4096 Mar 10  2018 node_modules
drwxrwxr-x 11 len len     4096 Apr  3 16:36 NopSCADlib
-rw-r--r--  1 len relaxo 17668 Jan 22  2014 parametric_involute_gear_v5.0.scad
drwxrwxr-x  2 len relaxo  4096 Mar 22  2017 Parametric_Molds
drwxrwxr-x  3 len len     4096 Oct  9  2019 pulleys
drwxrwxr-x  5 len relaxo  4096 Feb 13  2016 relativity.scad
lrwxrwxrwx  1 len relaxo    50 May 11  2011 ruler.stl -> /home/len/objects/thingiverse/ruler-6823/ruler.stl
drwxrwxr-x  3 len relaxo  4096 May  3  2017 scad-utils
drwxrwxr-x  3 len relaxo  4096 Jul 12  2014 syvwlch
drwxrwxr-x  3 len relaxo  4096 Mar  7  2018 threads

Is it also possible to log when the server tries to search within the library path?

@Leathong
Copy link
Owner

Leathong commented May 8, 2022

@Lenbok Your include statement is like <BOSL2/somthing> or not include the subdirectory "BOSL2"?

You can add the log at this line

let inccode = match self.code.get(&inc) {

@Lenbok
Copy link
Contributor Author

Lenbok commented May 8, 2022

Yes, my include statement includes the library name.

I added logging at 1146 to print inc and also at 1184 to log the value of text, but neither log statement gets triggered.

@Lenbok
Copy link
Contributor Author

Lenbok commented May 8, 2022

Ahhh, I narrowed it down, include is working, but use is not. I remember seeing something in the tree-sitter-openscad about treating those as separate node types, and checking now I see 0.3.0 you are using does do that (I also see there is now a 0.4.0). So there needs to be handling of use_statement node kind. I changed the handling at:

if node.kind() == "include_statement" {

to:

                    match node.kind() {
                        "use_statement" | "include_statement" => {
                            code.get_include_url(&node).map(|inc| {
                                include_vec.push(inc);
                            });
                        }
                        _ => (),
                    }

(remember, I don't know rust, I just copied the matching structure that was in the next section). And now my modules are being found! If you want I can open a PR, or you can just make the fix yourself.

Now that is solved, the other thing I noticed was that in dzhu's implementation, autocomplete candidates contain several extra candidates, both prefix matches and fuzzy matches (e.g. typing poly can suggest modules named e.g test_polyhole()). At least in emacs, the prefix matches come before the substring matches. Actually maybe dzhu's is returning all possible modules/variables, leaving the matching to emacs, while yours is doing some server side filtering?

@Lenbok Lenbok changed the title library resolution does not work on linux Library resolution does not work for use May 8, 2022
@Leathong
Copy link
Owner

Leathong commented May 9, 2022

@Lenbok I fixed it yesterday
875edd0

@Leathong Leathong closed this as completed May 9, 2022
@Leathong
Copy link
Owner

Leathong commented May 9, 2022

Actually maybe dzhu's is returning all possible modules/variables, leaving the matching to emacs, while yours is doing some server side filtering?

Yes, I did not return all symbols.

@Leathong
Copy link
Owner

Leathong commented May 9, 2022

@Lenbok I changed the server's behavior, just return all symbols.

@Lenbok
Copy link
Contributor Author

Lenbok commented May 9, 2022

Nice job, thanks!

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