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

Fuzzy counts work for /index.php vs / are not grouped together #272

Open
brendanheywood opened this issue Sep 15, 2022 · 3 comments
Open
Assignees
Labels
bug Something isn't working

Comments

@brendanheywood
Copy link
Contributor

brendanheywood commented Sep 15, 2022

We should fix this at the grouping level, any urls which are the same like / and /index.php should be merged into the same group, probably without the index.php

Special case, show the / at the root level as '/' and not as an empty string

@brendanheywood brendanheywood added the bug Something isn't working label Sep 15, 2022
@brendanheywood brendanheywood changed the title Fuzzy counts work for /index.php but don't work for / Fuzzy counts work for /index.php vs / are not grouped together Sep 15, 2022
@jaypha jaypha self-assigned this Sep 19, 2022
@jaypha
Copy link
Contributor

jaypha commented Sep 20, 2022

Is it safe to assume that the index is always 'index.php'?

@keevan
Copy link
Contributor

keevan commented Sep 20, 2022

Note that there may be cases where a php file is used but the path appears afterwards causing it to look like it was a folder.

An example of this might be the proxy pass or file used in our BBB fork.

But aside from these small instances, it's unlikely that a trailing slash is not considered an index page as that is kind of a common server page setup many sites use.

@jaypha
Copy link
Contributor

jaypha commented Sep 20, 2022

After consideration, I believe that is is simpler and safer to work with script names exclusively (for web requests).

Fragonite added a commit that referenced this issue Sep 5, 2023
Fragonite added a commit that referenced this issue Sep 5, 2023
Fragonite added a commit that referenced this issue Sep 5, 2023
Fragonite added a commit that referenced this issue Sep 5, 2023
Fragonite added a commit that referenced this issue Sep 5, 2023
Fragonite added a commit that referenced this issue Sep 5, 2023
Add @param description

Potential CI fix
Fragonite added a commit that referenced this issue Sep 6, 2023
[#272] Update unit tests for new logic
Fragonite added a commit that referenced this issue Sep 6, 2023
Fragonite added a commit that referenced this issue Sep 6, 2023
[#272] Attempt to solve CI error

[#272] Attempt to solve CI error
Fragonite added a commit that referenced this issue Sep 6, 2023
[#272] Update unit tests for new logic

[#272] Remove old logic (solves CI errors)

[#272] Solve more CI errors
brendanheywood pushed a commit that referenced this issue Sep 6, 2023
[#272] Update unit tests for new logic

[#272] Remove old logic (solves CI errors)

[#272] Solve more CI errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants