show tree edge before name block #489
Conversation
Codecov Report
@@ Coverage Diff @@
## master #489 +/- ##
==========================================
+ Coverage 84.46% 88.48% +4.02%
==========================================
Files 36 36
Lines 3456 3535 +79
==========================================
+ Hits 2919 3128 +209
+ Misses 537 407 -130
Continue to review full report at Codecov.
|
|
@zwpaper , thanks for taking this up. One thing that I am concerned with is handling Btw with this PR, if I am using date relative and the entire folder has a "smaller" date. The tree is misaligned. |
|
hi @meain , previously I have thought about making the so I decided to make it stick with the name block, IMHO the as the "smaller" date misaligned part, let me will dig deeper into it |
|
I initially did think of having tree as a block as well. We can default it to the start of the block list if Another issue with assigning Here is one other idea, I am not fully convinced of this myself but thought I would throw it out there. We can have another flag |
|
I just feel like that both the standing on the user case, is there really a case that users did not show the |
|
I don't think there is a strong usecase for blocks without
|
|
there is a bug in the original tree column calculation, I am working on it and then the last case you mentioned, nerther start nor end , columns will stay aligned. following your idea, if there is no name block, we add the tree prefix to the first column, making it align like a tree, while others align in column, ok? |
|
I have fixed the align problems including
now there should not be align problems. I will work onto show the tree edge when there is no |
zwpaper
force-pushed
the
dev-tree-long
branch
3 times, most recently
from
2255b28 to
729feaa
Compare
|
hi @meain , this PR should be ready now. with name: without name: |
| prefix: &str, | ||
| ) -> String { | ||
| let mut output = String::new(); | ||
| tree_depth_prefix: (usize, &str), |
There was a problem hiding this comment.
IMO, I think it would be cleaner to keep depth and prefix as two separate values itself.
There was a problem hiding this comment.
yes, I change this only because of the too_many_arguments warning, sigh...
and I can not figure out a better way to reduce the number of args, is there a better solution in your sight?
warning: this function has too many arguments (8/7), #[warn(too_many_arguments)] on by default
|
@zwpaper The |
Signed-off-by: zwPapEr <zw.paper@gmail.com>
Signed-off-by: zwPapEr <zw.paper@gmail.com>
Signed-off-by: zwPapEr <zw.paper@gmail.com>
Signed-off-by: zwPapEr <zw.paper@gmail.com>
Signed-off-by: zwPapEr <zw.paper@gmail.com>
Signed-off-by: zwPapEr <zw.paper@gmail.com>
Signed-off-by: zwPapEr <zw.paper@gmail.com>
|
@meain I have found the problem for the test, sorting is done before display, so that timesort is ignored. I have updated the code, and sorry for the force push, Have to rebase on to master |
|
Thanks for looking into the tree test. I am a bit busy now, will try to get the reviewed before next week. About force push, all good. In fact that is what you should be doing to fix issues in PRs. Just don't force push squashing every commit while working on the changes as that will make it harder to review in case someone has already taken a look at the previous commit. |
|
If the branch is not rebased on master you can also view the diff between the force pushes https://github.com/Peltoche/lsd/compare/5a558275fd37fe8efe0fa0e78bfc97da1b89c3dd..6a8ae5bdaabf1b941438e9962cc45fa60b56d58a |
| @@ -254,6 +256,8 @@ impl Colors { | |||
| m.insert(Elem::Links { valid: true }, Colour::Fixed(13)); | |||
| m.insert(Elem::Links { valid: false }, Colour::Fixed(245)); | |||
|
|
|||
| // TODO add this after we can use file to configure theme | |||
| // m.insert(Elem::TreeEdge, Colour::Fixed(44)); // DarkTurquoise | |||
There was a problem hiding this comment.
Wanted to do this a little while ago, I think we can make the color that is returned an Option and make this return None? I think we can take that up in another PR as well. This would let us remove the #[allow(dead_code)] above the struct definition.
Signed-off-by: zwPapEr <zw.paper@gmail.com>
Signed-off-by: zwPapEr <zw.paper@gmail.com>
|
LGTM |
show tree edge before name block
fix #468
TODO
cargo fmt