Skip to content

[Config][DependencyInjection] Uniformize trailing slash handling #40917

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 7, 2021
Merged

[Config][DependencyInjection] Uniformize trailing slash handling #40917

merged 1 commit into from
May 7, 2021

Conversation

dunglas
Copy link
Member

@dunglas dunglas commented Apr 22, 2021

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets n/a
License MIT
Doc PR n/a

Currently, the handling of trailing slashes in file loaders exclusion rules is inconsistent, which can create hard to debug issues.

Example:

services:
    App\:
        resource: '../src/'
        exclude:
            # This works
            - '../src/FooBar/DependencyInjection/'
            - '../src/FooBar/DependencyInjection'
            - '../src/FooBar/DependencyInjection/*'
            - '../src/*/DependencyInjection'
            - '../src/*/DependencyInjection/*'
           
            # This doesn't work
            - '../src/*/DependencyInjection/'

This PR fixes this subtle issue.

@carsonbot carsonbot added this to the 4.4 milestone Apr 22, 2021
@carsonbot carsonbot changed the title [DependencyInjection][Config] Uniformize trailing slash handling [Config][DependencyInjection] Uniformize trailing slash handling Apr 22, 2021
@dunglas dunglas force-pushed the fix/exclude-trailing-slash branch from 05e6278 to 0a39dd1 Compare April 22, 2021 22:56
@carsonbot
Copy link

Hey!

I think @tristanbes has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

@nicolas-grekas nicolas-grekas force-pushed the fix/exclude-trailing-slash branch from ddcfae9 to dc50aa3 Compare May 7, 2021 13:38
@nicolas-grekas
Copy link
Member

Thank you @dunglas.

@nicolas-grekas nicolas-grekas merged commit 0e738ef into symfony:4.4 May 7, 2021
This was referenced May 9, 2021
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

Assignees
No one assigned
Labels
Bug Config DependencyInjection Status: Reviewed
Projects
None yet
Milestone
4.4
Development

Successfully merging this pull request may close these issues.
3 participants
@dunglas @carsonbot @nicolas-grekas