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

Fixes empty favorite names for trailing slashes #11228

Merged
merged 1 commit into from
Sep 20, 2018

Conversation

weeman1337
Copy link
Member

The "function" for getting a readable directory name didn't work for trailing directory separators.

  • adds a test for that
  • extracts the directory naming function and adapts it to work with trailing separators

Relates to #11144

@weeman1337 weeman1337 added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Sep 15, 2018
*/
private function getDirDisplayName(string $dir): string {
if (substr($dir, -1) === '/') {
$dir = substr($dir, 0, -1);
Copy link
Member

Choose a reason for hiding this comment

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

rtrim($dir, '/') does all this in one nice method ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

cool - I will update it!

*/
private function getDirDisplayName(string $dir): string {
$dir = rtrim($dir, '/');
return substr($dir, strrpos($dir, '/') + 1, strlen($dir));
Copy link
Contributor

Choose a reason for hiding this comment

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

Does your tests pass when you replace substr with http://php.net/manual/en/function.basename.php?

Copy link
Member Author

Choose a reason for hiding this comment

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

They do. Good catch!

Signed-off-by: Michael Weimann <mail@michael-weimann.eu>
@nickvergessen nickvergessen added this to the Nextcloud 15 milestone Sep 19, 2018
@nickvergessen nickvergessen added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Sep 19, 2018
@nickvergessen nickvergessen merged commit 58e2818 into master Sep 20, 2018
@nickvergessen nickvergessen deleted the fix/11144/empty-favorite-names branch September 20, 2018 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants