Skip to content

don't specify ignorecase when calling isEquals and isEqualOrParent#75626

Merged
aeschli merged 6 commits intomasterfrom
aeschli/isEqualCaseSensitiveForNonFIle
Jul 11, 2019
Merged

don't specify ignorecase when calling isEquals and isEqualOrParent#75626
aeschli merged 6 commits intomasterfrom
aeschli/isEqualCaseSensitiveForNonFIle

Conversation

@aeschli
Copy link
Copy Markdown
Contributor

@aeschli aeschli commented Jun 17, 2019

No description provided.

@aeschli aeschli requested review from bpasero and sandy081 June 17, 2019 10:07
@aeschli
Copy link
Copy Markdown
Contributor Author

aeschli commented Jun 17, 2019 •
edited
Loading

@bpasero @sandy081 Please have a quick look as I changes some your of your files.

bpasero
bpasero approved these changes Jun 17, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@bpasero bpasero left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@aeschli one code change suggested.

Also: should we simply not allow to set ignoreCase on isEqual and isEqualOrParent? Being able to overrule this decision is only asking for trouble imho.

You also might want to do a quick scan of usages of isLinux to see if there are more places where we make these assumptions but where we are not using your resource helpers.

Comment thread src/vs/workbench/contrib/files/browser/editors/fileEditorTracker.ts
}

private getIndexOfPath(path: string, candidate: string): number {
private getIndexOfPath(path: string, candidate: string, ignoreCase: boolean): number {
Copy link
Copy Markdown
Member

@bpasero bpasero Jun 17, 2019

Choose a reason for hiding this comment

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

@aeschli I would just remove this parameter ignoreCase: boolean and inline it to resources.hasToIgnoreCase(resource)?

@aeschli aeschli changed the title make resources.isEqual case sensitive for non file URIs don't specify ignorecase when calling isEquals and isEqualOrParent Jul 11, 2019
@aeschli
Copy link
Copy Markdown
Contributor Author

aeschli commented Jul 11, 2019

I reverted the change of the default of ignorecase for remote resources.
The PR just fixes all callers to not define ignorecase.

@aeschli aeschli added this to the July 2019 milestone Jul 11, 2019
@bpasero
Copy link
Copy Markdown
Member

bpasero commented Jul 11, 2019

@aeschli possibly this one (explorerModel.ts) should get the same treatment as well.

@aeschli aeschli merged commit f3f82ca into master Jul 11, 2019
@aeschli
Copy link
Copy Markdown
Contributor Author

aeschli commented Jul 11, 2019

@bpasero Thanks, added 614db8b

@bpasero bpasero deleted the aeschli/isEqualCaseSensitiveForNonFIle branch July 15, 2019 06:08
@github-actions github-actions bot locked and limited conversation to collaborators Mar 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@bpasero bpasero bpasero approved these changes

@sandy081 sandy081 sandy081 approved these changes

Assignees

@aeschli aeschli

Labels

None yet

Projects

None yet

Milestone

July 2019

Development

Successfully merging this pull request may close these issues.

3 participants

@aeschli @bpasero @sandy081