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

Fix "folder_exclude_patterns" handling with relative project path #2142

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

rchl
Copy link
Member

@rchl rchl commented Dec 15, 2022

Handle relative project path.

@rchl rchl changed the title Fix issue with handling "folder_exclude_patterns" with relative project path Fix "folder_exclude_patterns" handling with relative project path Dec 15, 2022
@rchl
Copy link
Member Author

rchl commented Dec 15, 2022

Needs more work to handle Windows paths correctly.

Doesn't match in this case:

PATH E:\project\breakout\Library\PackageCache\[email protected]\Scripts\Runtime\TMP_FontAsset.cs
PATTERNS [
    '**/.svn/**',
    '**/.git/**',
    '**/.hg/**',
    '**/CVS/**',
    '**/.Trash/**',
    '**/.Trash-*/**',
    '**/E:\\project\\breakout/.svn/**',
    '**/E:\\project\\breakout/**/.svn/**',
    '**/E:\\project\\breakout/Library/**',
    '**/E:\\project\\breakout/**/Library/**',
    '**/E:\\project\\breakout/Logs/**',
    '**/E:\\project\\breakout/**/Logs/**',
    '**/E:\\project\\breakout/obj/**',
    '**/E:\\project\\breakout/**/obj/**',
    '**/E:\\project\\breakout/Temp/**',
    '**/E:\\project\\breakout/**/Temp/**'
]

@rchl
Copy link
Member Author

rchl commented Dec 15, 2022

With latest changes it's still not quite right (slashes are not uniform and the leading **/ is not compatible with Windows paths):

PATH E:\project\breakout\Library\PackageCache\[email protected]\Scripts\Runtime\TMP_FontAsset.cs
PATTERNS [
    '**/.svn/**',
    '**/.git/**',
    '**/.hg/**',
    '**/CVS/**',
    '**/.Trash/**',
    '**/.Trash-*/**',
    '**/E:\\project\\breakout/.svn/**',
    '**/E:\\project\\breakout/**/.svn/**',
    '**/E:\\project\\breakout/Library/**',
    '**/E:\\project\\breakout/**/Library/**',
    '**/E:\\project\\breakout/Logs/**',
    '**/E:\\project\\breakout/**/Logs/**',
    '**/E:\\project\\breakout/obj/**',
    '**/E:\\project\\breakout/**/obj/**',
    '**/E:\\project\\breakout/Temp/**',
    '**/E:\\project\\breakout/**/Temp/**'
]

@rchl
Copy link
Member Author

rchl commented Dec 21, 2022

I would like to release new version of LSP soon but I think this should be fixed first since it's a new functionality that wasn't released yet. Or alternatively we can revert support for this for now and re-add it later with the fix. I don't think I have patience to deal with file paths right now so might be better to revert for now.
@jwortmann

@jwortmann
Copy link
Member

I thought that I had verified my recent "pseudo-fix" for this, but I have tested again now and I can confirm that it doesn't work.

Also I've checked out this branch it still seems to be not fixed (I'm on Windows btw).

Not sure if it's necessary to revert the other PR, because it also never worked before, and at least it's probably a step in the right direction (sublime pattern -> glob pattern). You could just not mention it in the release notes instead maybe?

@rchl rchl marked this pull request as draft December 21, 2022 20:48
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.

2 participants