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

[FL-3891] Folder rename fails #3896

Merged
merged 8 commits into from
Sep 15, 2024
Merged

Conversation

portasynthinca3
Copy link
Member

@portasynthinca3 portasynthinca3 commented Sep 13, 2024

What's new

  • Split the storage_common_equivalent_path API function with a bool parameter at the end into two: storage_common_equivalent_path and storage_common_is_subdir
  • Fix the subdirectory checking functionality that setting the aforementioned parameter to true intends to achieve
  • By doing that, fix the bug where a folder rename would fail in some cases (e.g. /ext/test -> /ext/test-test)
  • A test for the aforementioned bug
  • Refactored storage tests a bit

Verification

  • Run unit tests

Or (in either qFlipper or the CLI):

  • Create a /ext/test folder
  • Try to rename the folder into /ext/test-test
  • Vierfy that the operation completes successfully

Remarks

  • I decided to split the function into two because previously it was not immediately clear that the semantics of the function change drastically if the last argument is set to true. Among other things, the order of the string parameters suddenly starts to matter, which is not clear from their names (path1 and path2). It still doesn't matter in the equivalence relation, but it does matter in the subdirectory relation.
  • That said, the implementation of both of these API functions is quite similar, so I decided not to add another message type in order to reduce code size.

Checklist (For Reviewer)

  • PR has description of feature/bug or link to Confluence/Jira task
  • Description contains actions to verify feature/bugfix
  • I've built this code, uploaded it to the device and verified feature/bugfix

Copy link

github-actions bot commented Sep 13, 2024

Compiled f7 firmware for commit b9b8765d:

@skotopes
Copy link
Member

Unit tests please

@portasynthinca3
Copy link
Member Author

portasynthinca3 commented Sep 13, 2024

Unit tests please

I already added unit tests for the folder rename functionality. Or are you referring to the new API?

@skotopes
Copy link
Member

Unit tests please

I already added unit tests for the folder rename functionality. Or are you referring to the new API?

yep

@portasynthinca3
Copy link
Member Author

yep

Done!

@portasynthinca3 portasynthinca3 added Bug Core+Services HAL, furi & core system services labels Sep 13, 2024
skotopes
skotopes previously approved these changes Sep 15, 2024
@skotopes skotopes merged commit 19a3736 into dev Sep 15, 2024
11 checks passed
@skotopes skotopes deleted the portasynthinca3/3891-storage-subdir branch September 15, 2024 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Core+Services HAL, furi & core system services
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants