Location.relativePath to use None value if rel path is not available #85

[Andrej730]

No description provided.

[LakshyAAAgrawal]

I am wondering, did you find this bug due to a crash?

[Andrej730]

I am wondering, did you find this bug due to a crash?

Yeah, was doing tests with multilspy and when I used request_definition on any symbol that comes from "site-packages" I got the error trying to get relative path - in my case script was on the disk "L:" and Python and "site-packages" were on disk "C:", system drive.

[LakshyAAAgrawal]

I think this is good to merge, but also, is there any way we could try to test this functionality, in such a manner that it fails without this change?

[Andrej730]

I think this is good to merge, but also, is there any way we could try to test this functionality, in such a manner that it fails without this change?

It is possible but it has system requirements - test can be only performed on Windows, since on Unix .drive is always the same and it's '', and it will need to have 2 different drives set up.

We can add a test for get_relative_path to make sure it checks that .drive is the same but I don't know how we can ensure with a test that request_ functions call get_relative_path, any ideas?

[Andrej730]

Got an idea for a test using unittest.mock (see sommit above). What do you think? @LakshyAAAgrawal

[Andrej730]

Okay, I guess even this kind of test only would work on Windows as PurePath.drive seems to always return "" on unix.

from pathlib import PurePath
path = "C:/test.py"
path = PurePath(path)
print(path) # C:\test.py

# prints "" on unix and "C:" on windows
print(repr(path.drive))

[LakshyAAAgrawal]

I think it is okay to skip this test. You can undo the testfile, and we can proceed with merge!

[Andrej730]

Removed the test.

[LakshyAAAgrawal]

Thanks a lot for the PR @Andrej730