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

Issue13 #38

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

Conversation

pascalfoerster
Copy link
Collaborator

fixes issue #13

All data in the current directory and all subdirectories are no longer loaded. Instead, now only the opened uvl files and the files imported there are loaded.

To make this possible, namespaces are no longer taken into account during import, but only the path with the file name.
If the uvl.json was opened and the corresponding uvl file was not loaded yet, it will be loaded afterward.

Hints:

  • The automatic completion during import for not-loaded files had to be implemented additionally. Since there are often problems with the file system with different operating systems, and it has only been tested with Linux so far, this still needs to be tested with Windows.

@pascalfoerster
Copy link
Collaborator Author

pascalfoerster commented Jun 27, 2023

What is the best way to test this?

  1. it is advantageous to have a folder open with as many uvl files as possible
  2. it is especially important to open uvl files that import other uvl files, these should be loaded from the server then. It is best to import uvl files with errors or warnings, because the errors are optimally captured in the problem bar.
  3. then the autocomplete for imports should be tested, so that it is checked whether files that are not loaded are displayed.
  4. if the uvl.json is opened but the corresponding uvl file is not loaded from the server yet, this should be done right after opening the JSON file and no error message should appear.
  5. you can test if the namespaces are ignored during import and it works only with the correct filename(path).

@ThiBruUU
Copy link
Collaborator

Do you have a folder with many uvl files that you can share with us. Maybe as a zip file?

@pascalfoerster
Copy link
Collaborator Author

@TobiUUlm
Copy link
Collaborator

I guess there are still (or again) issues with the Windows File System.
The imports don't work at all and can not be resolved.

issue13_test01

Could you please tell us, where exactly file accesses happen? That way we can resolve the issue on our systems.

Additionally, where did you find files with a lot of imports? I think the repo you mentioned does not have a single file with any imports.

@pascalfoerster
Copy link
Collaborator Author

The problem shown should give these error messages because namespaces are ignored. It should be Mendonca2009 or Perreira2016 as the import, then it should work. Auto-completion should actually show the correct names. If there is a problem with the file system there too, this is handled in completion::994 with the method from cache::273 all_sub_files(...).

If you still get error messages, then the file access is done in pipeline::506 in the method loade_imports(...).

I also don't have an UVL file with many imports.

@TobiUUlm
Copy link
Collaborator

  1. it is advantageous to have a folder open with as many uvl files as possible

i opened the suggested files from the big uvl collection

  1. it is especially important to open uvl files that import other uvl files, these should be loaded from the server then. It is best to import uvl files with errors or warnings, because the errors are optimally captured in the problem bar.

Seems to work

  1. then the autocomplete for imports should be tested, so that it is checked whether files that are not loaded are displayed.

Worked only if the files have been opened before once. Probably a windows issue, i suspect.

EDIT: I think I fixed it on windows with the latest commit.
However, there's still an issue with the autocompletion: it suggests finance\bank\AlHajjaji2019 but it should be finance.bank.AlHajjaji2019 with dots, am I right?
Otherwise it won't be resolved as an import.

  1. if the uvl.json is opened but the corresponding uvl file is not loaded from the server yet, this should be done right after opening the JSON file and no error message should appear.

I think this is a windows issue after all:
2023-06-30 10_33_25- Extension Development Host  Pereira2016 uvl json - uvl - Visual Studio Code

EDIT: I think i "fixed" this issue but it's not pretty at all and I don't know if it works properly.
Currently, it only works if the corresponding .uvl file is also explicitly opened in the editor.
Is it the same on Linux? @pascalfoerster

  1. you can test if the namespaces are ignored during import and it works only with the correct filename(path).

Namespaces are not recognized correctly. It says namespace already defined for most of them (oddly not for all)
2023-06-30 10_34_40- Extension Development Host  Schulze2012 uvl - uvl - Visual Studio Code

name,
path.clone(),
len,
TextOP::Put(path),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you should replace the slashes with dots here for proper completion

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes should actually be so, just made a mistake that should be fixed in the next push

path.extend_from_slice(&ns.names);
}
let path = uri_to_path(&uri).unwrap();
// without this Code namespaces are ignored for imports and the normal path is used
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's a good idea to keep this within the code.
Does this work like a "switch" or does it need additional changes to make the import work like before?

Copy link
Collaborator

@ThiBruUU ThiBruUU left a comment

Choose a reason for hiding this comment

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

LGTM

@pascalfoerster
Copy link
Collaborator Author

However, there's still an issue with the autocompletion: it suggests finance\bank\AlHajjaji2019 but it should be finance.bank.AlHajjaji2019 with dots, am I right?

you are right

I think i "fixed" this issue but it's not pretty at all and I don't know if it works properly.
Currently, it only works if the corresponding .uvl file is also explicitly opened in the editor.
Is it the same on Linux?

In Linux it works even without the UVL file loaded

Namespaces are not recognized correctly. It says namespace already defined for most of them (oddly not for all)

The next push should fix this error

@TobiUUlm
Copy link
Collaborator

TobiUUlm commented Jul 6, 2023

I think i "fixed" this issue but it's not pretty at all and I don't know if it works properly.
Currently, it only works if the corresponding .uvl file is also explicitly opened in the editor.
Is it the same on Linux?

In Linux it works even without the UVL file loaded

Now I can not reproduce it anymore. With a small sample file the configuration works without problems. Maybe it takes too long to index all files.

Waiting for the next push, after that it's good to merge.

@TobiUUlm TobiUUlm mentioned this pull request Jul 6, 2023
@pascalfoerster pascalfoerster requested a review from felixrieg July 25, 2023 10:25
@felixrieg
Copy link
Collaborator

I did as suggested and had some probelms:

it is especially important to open uvl files that import other uvl files, these should be loaded from the server then. It is best to import uvl files with errors or warnings, because the errors are optimally captured in the problem bar.

I have the same issue as tobi had: Worked only if the files have been opened before once. Probably a windows issue, i suspect.

then the autocomplete for imports should be tested, so that it is checked whether files that are not loaded are displayed.

This worked but when i initially open my ide in a folder all the imports get taged as: unresolved import. As soon as I retype one letter all unresolved import errors disappear, but errors in those files still not show.

I also have some really weird bugs:

  • I cant configure a uvl file with imports, It says: Loading Model and the Log says: successfully cancelled request with ID: 60
  • If I open IDE in a uvl file with imports, there is no error detection in any of the imported files until I open ALL the imported files first. Other files still have error detection.
  • I don't know if this should be a separate issue: If I delete a File with a error and then delete said file, the error is still shown at the bottom of vs code.

This said, I don't have any of the above mentioned issues on MacOS, except for the delete file issue.

I have the log for a uvl file with 3 imports, one of which has an error:

INFO [uvls] UVLS start
INFO [uvls] create service
INFO [uvls::core::pipeline] started link handler
INFO [uvls::core::pipeline] started link execute
INFO [uvls::webview] Starting web handler
INFO [uvls::smt] Check SMT
INFO [uvls] received did_open Url { scheme: "file", cannot_be_a_base: false, username: "", password: None, host: None, port: None, path: "/c%3A/Users/felix/Documents/Programmieren/UVL/test.uvl", query: None, fragment: None }
INFO [uvls::core::semantic] file id: test.uvl
INFO [uvls] done did_open
INFO [uvls::core::parse] Parsed in 12.3027ms
INFO [uvls::core::pipeline] started draft handler file:///c%3A/Users/felix/Documents/Programmieren/UVL/test.uvl
INFO [uvls::core::pipeline] update red tree file:///c%3A/Users/felix/Documents/Programmieren/UVL/test.uvl
INFO [uvls::core::check]  check sanity in 499µs
INFO [uvls::core::pipeline] waited 377.4µs for draft
INFO [uvls::core::pipeline] link prepare
INFO [uvls::core::pipeline] link execute
INFO [uvls::core::cache] Cant find [u!("error")] 
INFO [uvls::core::cache] Cant find [u!("test3")] 
INFO [uvls::core::cache] Cant find [u!("test2")] 
INFO [uvls::core::cache] updating cache dirty {test.uvl}
INFO [uvls::smt] Check SMT
INFO [uvls] update inlays Range { start: Position { line: 0, character: 0 }, end: Position { line: 12, character: 10 } }
INFO [uvls::ide::inlays] inlays get
INFO [uvls::ide::inlays] inlays done
INFO [uvls::core::semantic] file id: test.uvl

@felixrieg
Copy link
Collaborator

I played around with this feature and at first it worked as intended, but then I came across a weird bug.

I made a screen recording for better understanding.
This bug happened for me on windows 11 and Mac.
First I made two error files (error.uvl and error2.uvl) each containing an error.
Then I made an import file importing some arbitrary files and error.uvl. I restarted the server and when trying to import error2.uvl as well in this file it stopped working until I remove the last import.

Screen.Recording.2023-09-01.at.16.14.08.mov

Here are logs after typing the import and I also clicked on the error2.uvl afterwards (Not the logs from the video):

INFO [uvls::core::parse] Parsed in 1.50175ms
INFO [uvls::core::pipeline] update red tree file:///Volumes/SSD-ext/RANDOM/import.uvl
INFO [uvls::core::check]  check sanity in 204.667µs
INFO [uvls::core::semantic] file id: Test2.uvl
INFO [uvls::core::semantic] file id: error.uvl
INFO [uvls::core::semantic] file id: error2.uvl
INFO [uvls::core::parse] Parsed in 254.583µs
INFO [uvls::core::pipeline] started draft handler file:///Volumes/SSD-ext/RANDOM/error2.uvl
INFO [uvls::core::pipeline] update red tree file:///Volumes/SSD-ext/RANDOM/error2.uvl
INFO [uvls::core::check]  check sanity in 87.916µs
INFO [uvls::core::pipeline] link prepare
INFO [uvls::core::pipeline] link execute
INFO [uvls::core::cache] updating cache dirty {error2.uvl, import.uvl}
INFO [uvls::smt] Check SMT
INFO [uvls::smt::smt_lib] expr Var(14)
INFO [uvls::smt::smt_lib] model to string  in 194.166µs
INFO [uvls::smt] create model: 610.709µs
INFO [uvls::core::semantic] file id: import.uvl
INFO [uvls::core::pipeline] waited 143.709µs for draft
INFO [uvls::core::semantic] file id: import.uvl
INFO [uvls::core::pipeline] waited 83.125µs for root
INFO [uvls::ide::color] Semantic highlight took 701.75µs
INFO [uvls::core::pipeline] waited 215.459µs for draft
INFO [uvls] update inlays Range { start: Position { line: 0, character: 0 }, end: Position { line: 26, character: 4 } }
INFO [uvls::ide::inlays] inlays get
INFO [uvls::ide::inlays] inlays done
INFO [uvls] received did_open Url { scheme: "file", cannot_be_a_base: false, username: "", password: None, host: None, port: None, path: "/Volumes/SSD-ext/RANDOM/error2.uvl", query: None, fragment: None }
INFO [uvls::core::semantic] file id: error2.uvl
INFO [uvls] done did_open
INFO [uvls::core::parse] Parsed in 118.25µs
INFO [uvls::core::pipeline] started draft handler file:///Volumes/SSD-ext/RANDOM/error2.uvl
INFO [uvls::core::pipeline] update red tree file:///Volumes/SSD-ext/RANDOM/error2.uvl
INFO [uvls::core::check]  check sanity in 38.25µs
INFO [uvls::core::pipeline] waited 95.917µs for draft
INFO [uvls::core::semantic] file id: error2.uvl
INFO [uvls::core::pipeline] waited 51.875µs for root
INFO [uvls::ide::color] Semantic highlight took 89.417µs
INFO [uvls::core::pipeline] waited 43.875µs for draft
INFO [uvls::core::pipeline] link prepare
INFO [uvls::core::pipeline] link execute
INFO [uvls::core::cache] updating cache dirty {error2.uvl, import.uvl}
INFO [uvls] update inlays Range { start: Position { line: 0, character: 0 }, end: Position { line: 6, character: 10 } }
INFO [uvls::smt] Check SMT
INFO [uvls::ide::inlays] inlays get
INFO [uvls::ide::inlays] inlays done
INFO [uvls::smt::smt_lib] expr Var(14)
INFO [uvls::smt::smt_lib] model to string  in 114.084µs
INFO [uvls::smt] create model: 773.708µs
INFO [uvls::core::semantic] file id: error2.uvl

@pascalfoerster pascalfoerster marked this pull request as draft September 4, 2023 14:08
@pascalfoerster pascalfoerster marked this pull request as ready for review September 26, 2023 10:13
@SundermannC SundermannC self-requested a review September 26, 2023 10:14
@felixrieg
Copy link
Collaborator

LGTM!

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

Successfully merging this pull request may close these issues.

4 participants