Introduce GetFileArg and use it where possible #24274

1. 
   prusnak commented at 6:39 pm on February 5, 2022: contributor

   Follow-up to #24265

   Marking as draft until #24265 is merged as this PR builds on top of that PR

2. prusnak force-pushed on Feb 5, 2022
3. DrahtBot added the label Utils/log/libs on Feb 5, 2022
4. DrahtBot added the label Wallet on Feb 5, 2022
5. 
   DrahtBot commented at 9:26 pm on February 5, 2022: member

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #24266 (util: Avoid buggy std::filesystem:::create_directories() call by hebasto)
   • #23732 (refactor: Remove gArgs from bdb.h and sqlite.h by kiminuo)
   • #17783 (util: Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior by ryanofsky)
   • #17581 (refactor: Remove settings merge reverse precedence code by ryanofsky)
   • #17580 (refactor: Add ALLOW_LIST flags and enforce usage in CheckArgFlags by ryanofsky)
   • #17493 (util: Forbid ambiguous multiple assignments in config file by ryanofsky)
   • #16545 (refactor: Implement missing error checking for ArgsManager flags by ryanofsky)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

6. in src/util/system.cpp:389 in f49b27b1c6 outdated

   385@@ -399,6 +386,23 @@ std::optional<unsigned int> ArgsManager::GetArgFlags(const std::string& name) co
   386     return std::nullopt;
   387 }
   388 
   389+fs::path ArgsManager::GetFileArg(std::string pathlike_arg, const fs::path& pathDefault) const
   

   ---

   ---

   

   kiminuo commented at 8:03 am on February 6, 2022:

   pathlike_arg is in snake case notation, pathDefault is in camel case notation.

   ---

   

   prusnak commented at 9:23 am on February 6, 2022:

   Right. Left review on the PR this PR builds on: #24265#pullrequestreview-874013646

   ---

   

   prusnak commented at 9:54 am on February 6, 2022:

   Addressed in c71437c93611094aa57eb7da0e166159ff128bdd
7. prusnak force-pushed on Feb 6, 2022
8. DrahtBot added the label Needs rebase on Feb 8, 2022
9. prusnak force-pushed on Feb 9, 2022
10. DrahtBot removed the label Needs rebase on Feb 9, 2022
11. Introduce GetFileArg 42688109b3
12. Use GetFileArg where possible 7db1e1fa73
13. prusnak force-pushed on Feb 9, 2022
14. prusnak marked this as ready for review on Feb 9, 2022
15. 
    ryanofsky commented at 11:14 pm on February 9, 2022: member

    Concept ACK, but I think PR description should be clearer. Also, I think it would be better to drop the first commit 42688109b38063902f4c2621db3e465a3c9b2218 here, and substitute commit b00c0c2a73b3c443f78b74eee60ff3e401b0269f from #24306. Main reason is that I think it is confusing to have separate GetPathArg and GetFileArg methods which are exactly the same except one strips trailing slashes and one doesn’t. I don’t think it’s likely we will have a need for the one which preserves trailing slashes. The commit from the other PR also adds unit test coverage.
16. 
    prusnak commented at 10:27 pm on February 10, 2022: contributor

    Let’s close this in favor of https://github.com/bitcoin/bitcoin/pull/24306
17. prusnak closed this on Feb 10, 2022

    ---

18. prusnak deleted the branch on Feb 10, 2022
19. MarcoFalke referenced this in commit 6687bb24ae on Mar 7, 2022
20. DrahtBot locked this on Feb 10, 2023

Contributors
prusnak DrahtBot kiminuo ryanofsky

Labels
Wallet Utils/log/libs