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

Git integration (issue #7) #495

Closed
wants to merge 23 commits into from
Closed

Conversation

hpwxf
Copy link
Contributor

@hpwxf hpwxf commented Mar 7, 2021

Add git integration (cf issue #7)

Use git2-rs to get git info. Unfortunately, it is not available on all targets. To enable it, use cargo build --features=git.
In travis config, this feature is enabled using a new FEATURES environment variable (where it is not set, git2-rs does not compile; cf https://travis-ci.com/github/hpwxf/lsd/builds/219208311).
git_stub.rs file contains a minimal set of empty/disabled features to minimize condition compilation in other files.

  • New git block is located by default on the left of name block and its location can be set using --blocks option
  • Use lsd --git --long to enable git status info (only available on long display)
  • Additional feature: sort by git status using lsd --sort=git --long
  • Default display use icons. Text display is also available.

Snapshot 3
Snapshot

NB: crate log has also been defined in Cargo.toml to provide debug feature. release_max_level_error feature disables log macros in release mode.

  • Use cargo fmt
  • Add necessary tests
  • Add changelog entry

@hpwxf hpwxf requested review from meain and Peltoche as code owners March 7, 2021 22:23
@codecov-io
Copy link

codecov-io commented Mar 7, 2021

Codecov Report

Merging #495 (c7baca2) into master (7254816) will increase coverage by 0.27%.
The diff coverage is 85.95%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #495      +/-   ##
==========================================
+ Coverage   88.48%   88.76%   +0.27%     
==========================================
  Files          36       40       +4     
  Lines        3535     3951     +416     
==========================================
+ Hits         3128     3507     +379     
- Misses        407      444      +37     
Impacted Files Coverage Δ
src/config_file.rs 69.23% <ø> (ø)
src/core.rs 0.00% <0.00%> (ø)
src/flags.rs 100.00% <ø> (ø)
src/git_stub.rs 0.00% <0.00%> (ø)
src/main.rs 10.00% <ø> (ø)
src/sort.rs 96.02% <0.00%> (-1.95%) ⬇️
src/meta/git_file_status.rs 25.00% <25.00%> (ø)
src/icon.rs 97.05% <33.33%> (-0.57%) ⬇️
src/meta/mod.rs 41.66% <62.50%> (+2.19%) ⬆️
src/color.rs 78.94% <79.59%> (+0.30%) ⬆️
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7254816...c7baca2. Read the comment docs.

@hpwxf
Copy link
Contributor Author

hpwxf commented Mar 9, 2021

I found a bug when I use options -a, -l and --git together. Processing of . and .. directories do something strange which breaks the layout. I will fix it (and ok with -Al --git)

@hpwxf hpwxf force-pushed the git-integration-issue-7 branch from f197535 to 4556325 Compare March 23, 2021 12:53
@hpwxf
Copy link
Contributor Author

hpwxf commented Mar 23, 2021

Since git2 has inconsistent behaviors on

  • arm-unknown-linux-gnueabihf
  • i686-pc-windows-msvc
  • x86_64-pc-windows-gnu
  • x86_64-pc-windows-msvc
    and a compilation error on
  • i686-pc-windows-gnu
    I will investigate what happens in git2.

hpwxf added 22 commits April 3, 2021 20:27
* Enabled using `cargo build --feature=git`
* git block is by default on the left of name block
* use `lsd --git --long` to enable git status info (only available on long display)
* sort by git status using `lsd --sort=git --long`
Test enum completeness in color map using enum iteration from strum macros
Parent directory git status will not fetch another parent repository if different of the current one.
color::tests::test_elem_map_completeness verify the consistency between between Elem value and theme.

Elem::TreeEdge could be re-enabled when its theme will ok.
@hpwxf hpwxf force-pushed the git-integration-issue-7 branch from 4556325 to c7baca2 Compare April 3, 2021 20:40
@hpwxf
Copy link
Contributor Author

hpwxf commented Apr 3, 2021

  • Since git2-rs requires rust 1.45, I had to upgrade RUST_MIN_SRV to 1.45
  • Using a complex conditional compilation, git feature is always enabled except on
    • arm-unknown-linux-gnueabihf (tests fail in git2-rs)
    • i686-pc-windows-gnu (compilation fails in git2-rs)
  • new tests have been done to improve overall coverage (not only in new code for git feature)

@hpwxf
Copy link
Contributor Author

hpwxf commented Apr 3, 2021

NB: to support conditional compilation, I used a stub file (git_stub.rs) to provide minimal objects to avoid function signature to change or to be also conditional. This technique could reduce the patch coverage since only one version is covered by tests.

@meain
Copy link
Member

meain commented Apr 20, 2021

Hey, sorry about the delay. I have been pretty busy the past few weeks. Will try to get this reviewed by this week.

@gwennlbh
Copy link

If I may give feedback, I've been using this branch on my machine for more than a week and found no issues whatsoever.

@BrandonStaab
Copy link

There is inconsistent behavior when listing outside of a repo vs inside a repo as compared with the yellow squares. Similarly using --tree outside of a repo doesn't display status.

@hpwxf
Copy link
Contributor Author

hpwxf commented Nov 11, 2021

There is inconsistent behavior when listing outside of a repo vs inside a repo as compared with the yellow squares. Similarly using --tree outside of a repo doesn't display status.

Thank you for your comment. I will check ASAP (probably next week) what happens in this case and how to fix it.

@BrandonStaab
Copy link

Thank you for your comment. I will check ASAP (probably next week) what happens in this case and how to fix it.

Also I just noticed that the directory .. shows a question mark, but it isn't a repo so it shouldn't have any styling. I put a green box around the mark in question.

@hpwxf hpwxf mentioned this pull request Sep 11, 2022
@hpwxf
Copy link
Contributor Author

hpwxf commented Sep 11, 2022

  • Since git2-rs requires rust 1.45, I had to upgrade RUST_MIN_SRV to 1.45

  • Using a complex conditional compilation, git feature is always enabled except on

    • arm-unknown-linux-gnueabihf (tests fail in git2-rs)
    • i686-pc-windows-gnu (compilation fails in git2-rs)
  • new tests have been done to improve overall coverage (not only in new code for git feature)

May be https://crates.io/crates/git-repository could be a simple replacement to git2-rs.

@meain
Copy link
Member

meain commented Sep 11, 2022

Hey @hpwxf , sorry for kinda abandoning this PR. If you are still interested and won't mind fixing up the conflicts and addressing the previous issue we can get this in.

@meain
Copy link
Member

meain commented Sep 11, 2022

May be crates.io/crates/git-repository could be a simple replacement to git2-rs.

What do you think are the primary advantages of switching to this?

@meain
Copy link
Member

meain commented Oct 9, 2022

I'm closing this, but if folks want to continue on this, we can open this back up and continue work on this. Sorry that the work you did could not get merged.

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.

5 participants