feat(git): improve readability and maintainability
Description
- Improve readability and maintainability.
- Default usage of
nf-oct-diffseries icons from Nerdfonts and terminal colors (instead ofRGBcolors).
This PR contains too much content, which makes it difficult to diff. Is it possible to split it into several smaller PRs?
Sorry, but I'm afraid this would be quite difficult, as it involves an almost complete refactor with various parts intertwined. However, I can provide some technical details:
- First, I removed
propagate_downand the status marked as30, which was very difficult to understand without comments. To restore the retrieval of theignoredstatus related to this, I wrote theget_kindfunction. This function searches upwards layer by layer and makes the file (or folder)ignoredif its parent folder is marked asignored. - Secondly, to facilitate maintenance and improve code readability, I introduced the
Statusenum along with a series of type annotations. - Thirdly, I added the detection of the
renamedstatus, mainly involving lines113-116, which are easy to understand.
I have no classes tomorrow afternoon, so I can try to split it into multiple commits. However, it might be a bit late if you're already working on reviewing this PR :(
If you don't mind, I will force push to the PR after I finish. The new commits include the following changes:
- Reverted the configuration options for
iconsandstylesinsetup(but I have kept the related commits locally and am ready to patch them anytime — trust me, people tend to prefer keeping configurations together). - Used
THEMEconsistently in both theREADMEandmain.lua, as I still couldn't get different variables to work together (THEMEininit.luaandthinmain.lua) — as I mentioned above. (with the latest commit 38d5b2f) - Unified variable names.
- Added caching in
get_status(previouslyget_kind) to improve performance.
Sorry, I didn't realize that. I will try to revert it. (Perhaps there's a better way to handle this? The introduction of 30 has increased the complexity and difficulty of understanding the code.)
IMO this PR should just focus on adding the new feature and keep the changes as small as possible so it’s easier to review. Improving code readability is great, but it might be worth doing that in a separate PR where we can dig into it a bit more and discuss it further.
On Mon, Mar 3, 2025 at 3:04 PM PFiS @.***> wrote:
Sorry, I didn't realize that. I will try to revert it. (Perhaps there's a better way to handle this? The introduction of 30 has increased the complexity and difficulty of understanding the code.)
— Reply to this email directly, view it on GitHub https://github.com/yazi-rs/plugins/pull/78#issuecomment-2693457450, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEFWFIHLZSUJZ5BOL6EAUCL2SP5GZAVCNFSM6AAAAABYE6ER3WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMOJTGQ2TONBVGA . You are receiving this because you commented.Message ID: @.***> [image: PFiS1737]PFiS1737 left a comment (yazi-rs/plugins#78) https://github.com/yazi-rs/plugins/pull/78#issuecomment-2693457450
Sorry, I didn't realize that. I will try to revert it. (Perhaps there's a better way to handle this? The introduction of 30 has increased the complexity and difficulty of understanding the code.)
— Reply to this email directly, view it on GitHub https://github.com/yazi-rs/plugins/pull/78#issuecomment-2693457450, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEFWFIHLZSUJZ5BOL6EAUCL2SP5GZAVCNFSM6AAAAABYE6ER3WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMOJTGQ2TONBVGA . You are receiving this because you commented.Message ID: @.***>
Sorry for wasting your time, and please forgive my unfamiliarity. I will revert the commits introducing the new feature, making this PR solely focused on discussing improvements to code readability and maintainability.
The rollback is complete, and I have translated the original logic according to the latest type definitions. I will continue to explore how to remove the __ignored and "" entries in dirs_to_repo, which are confusing. Or I will try adding comments.
Well, I failed — I couldn't come up with any method other than using an additional flag.
However, I made some optimizations:
- remove the conditional checks in
bubble_upandpropagate_downsince they always pass - add comments and use named flags to make the code a bit easier to understand
- and some minor adjustments
I have only reviewed part of it, and it is still very difficult to review as the different changes are intertwined and this PR refactored almost the entire plugin, and I am not sure how to correctly review them.
I'll try to split these changes into several smaller PRs later and request you review them. I will add you as a collaborator to make sure you get the credits.
Sincerely appreciate your efforts and thank you for your patience these days. I will do my best to fulfill my part.
Hi, I'm planning to submit separate PRs over the next few days to complete the updates:
- [x] Simplify the code — mainly by removing unnecessary logic, possibly with some variable name changes. (#80, #81, #82)
- [x] Adjust the priority of codes (
untrackedhigher, perhaps needs discussion?). (#83) - [x] Add ~~type declarations and~~ some comments. (#84)
- [x] Modify the default
stylesandsignsto use terminal colors and consistent series icons. (#86) - [ ] Add support for
renamedstatus. (#88) - [ ] Add highlight support for list entries.
Today's will be the first.
You can review them and give feedback whenever you have time, and I'll submit the next one once the previous one is merged.