Skip to content

Conversation

@logresearch
Copy link
Contributor

@logresearch logresearch commented Jul 2, 2024

Description

This pull request addresses the issues with the logging statements in the fromContentUri method. The changes improve the clarity and context of the log messages when navigating to file paths.

The original code is confusing to understand.

File pathFile = new File(uri.getPath().substring(FILE_PROVIDER_PREFIX.length() + 1));
    if (!pathFile.exists()) {
      LOG.warn("failed to navigate to path {}", pathFile.getPath());
      pathFile = new File(uri.getPath());
      LOG.warn("trying to navigate to path {}", pathFile.getPath());
    }
    return pathFile;

Changes:

  1. Initial Path Navigation Failure Log:

    • Original:
      LOG.warn("failed to navigate to path {}", pathFile.getPath());
    • Improved:
      LOG.warn("Failed to navigate to the initial path: {}", pathFile.getPath());
    • Reason: Provides more context by indicating that this is the initial path navigation attempt.
  2. Fallback Path Navigation Log:

    • Original:
      LOG.warn("trying to navigate to path {}", pathFile.getPath());
    • Improved:
      LOG.warn("Attempting to navigate to the fallback path: {}", pathFile.getPath());
    • Reason: Clarifies that this is a second attempt to navigate to a fallback path.

These improvements will help in better understanding and diagnosing issues related to file path navigation failures.

Automatic tests

  • Added test cases

Manual tests

  • Done

Build tasks success

Successfully running following tasks on local:

  • ./gradlew assembledebug
  • ./gradlew spotlessCheck

@VishnuSanal
Copy link
Member

thanks for your interest in contributing to amaze. but, I am afraid I don't think this is a mandatory change to be made.

@VishnuSanal VishnuSanal closed this Jul 2, 2024
@logresearch
Copy link
Contributor Author

thanks for your interest in contributing to amaze. but, I am afraid I don't think this is a mandatory change to be made.

Hi @VishnuSanal ,

Thanks for your quick reply.
We are doing research on automatically maintain the quality of logging statement.
We would be grateful if the developers for popular repositories think it's useful.
We are looking forward for your opinion for automatically LS quality maintenance!

@VishalNehra
Copy link
Member

Looks fine to me. Any code improvements are welcome whether it's for readability or actually fixes / issues.

@VishalNehra VishalNehra reopened this Jul 2, 2024
@logresearch
Copy link
Contributor Author

logresearch commented Jul 3, 2024

Hi @VishalNehra,

Thanks for your positive reply!
Really hope my contribution can make AmazeFileManagermore perfect, though the contribution is incremental.
Based on the feedback, I further reviewed some source code and logging messages and found the following inaccurate expression.

https://github.com/TeamAmaze/AmazeFileManager/blob/release/4.0/app/src/main/java/com/amaze/filemanager/asynchronous/loaders/AppListLoader.java#L87
LOG.warn("faield to find android package name while loading apps list", e);
The faield is a typo, which should be failed

Also the same in https://github.com/TeamAmaze/AmazeFileManager/blob/release/4.0/app/src/main/java/com/amaze/filemanager/asynchronous/loaders/AppListLoader.java#L102
faield -> failed

We update them in this PR together.

@VishalNehra VishalNehra merged commit cb2b2e3 into TeamAmaze:release/4.0 Jul 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants