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

WIP: 190: Configurable diffs #201

Closed
wants to merge 11 commits into from
Closed

Conversation

jesseduffield
Copy link
Owner

@jesseduffield jesseduffield commented Aug 21, 2018

This allows for users to specify a git diff template which will have its placeholder values populated at runtime.

TODO: benchmarking to see if this slows things down for the regular git diff
TODO: don't use runDirectCommand unless there is a user config option

@jesseduffield jesseduffield changed the title 190: Configurable diffs WIP: 190: Configurable diffs Aug 21, 2018
@snoblenet
Copy link

snoblenet commented Nov 29, 2018

Thanks for this. Can we get configuration instructions please?

My ‎⁨~/Library/⁨Application Support/⁨jesseduffield⁩/lazygit/config.yml is as per below and I have icdiff installed, but diffs are not appearing as side-by-side. Thanks.

reporting: "on"
git:
  fileDiffTemplate: 'git difftool --no-prompt --extcmd="icdiff --cols={{width}}" {{args}} -- {{filename}}'

@jesseduffield
Copy link
Owner Author

@snoblenet I would have a look at your logs and see what command it's using. I use this alias when working with local branches:
alias gg="go install github.com/jesseduffield/lazygit && lazygit"
and so if you want to get debugging you would just use checkout this branch and do gg -debug.
Then in your lazygit directory you'll have a development.log file which among other things will print every git command.
In my case, I've got this in my logs:

time="2018-12-02T10:01:44+11:00" level=info msg=RunDirectCommand command="git difftool --no-prompt --extcmd=\"icdiff --cols=76\"  -- -- \"pkg/commands/git.go\""

logrus has added those backslashes, so after removing those, I get:

git difftool --no-prompt --extcmd="icdiff --cols=76"  -- -- "pkg/commands/git.go"

Which gives me a side-by-side diff if I paste that directly into my terminal.

Having said all that, looks like the original command template I used doesn't need the last --, and that causes issues when trying to diff an untracked file. I've just pushed a commit that updates the example command template.

Let me know if that helps :)

@snoblenet
Copy link

Do I have to check out a specific branch for logging? As I tried just logging with my current installation using lazygit -debug and nothing happened.

@mjarkk
Copy link
Contributor

mjarkk commented Dec 3, 2018

@snoblenet you don't have to use a specific branch for logging..
If you use -debug it creates a development.log with the logs form the application
Every log is added to the end of that file

@snoblenet
Copy link

Ah sorry I was expecting development.log to be alongside config.yml, not at the root of my git project.

Some output from cat development.log | grep diff:

time="2018-12-03T09:31:32+11:00" level=info msg="[git diff --color -- src/routes/user_management/routes/users/component.jsx]" buildDate="2018-10-23T22:33:44Z" commit=c6da4c8a47a45a96beb2de4fbe5b98cfb87446b4 debug=true version=0.5
time="2018-12-03T09:31:36+11:00" level=info msg=RunCommand buildDate="2018-10-23T22:33:44Z" command="git diff --color  -- 'src/routes/user_management/routes/users/component.jsx'" commit=c6da4c8a47a45a96beb2de4fbe5b98cfb87446b4 debug=true version=0.5
time="2018-12-03T09:31:36+11:00" level=info msg="[git diff --color -- src/routes/user_management/routes/users/component.jsx]" buildDate="2018-10-23T22:33:44Z" commit=c6da4c8a47a45a96beb2de4fbe5b98cfb87446b4 debug=true version=0.5

@snoblenet
Copy link

I was keen to test this branch so that it can be merged.

However, I have encountered a roadblock:

~/code → brew uninstall --force lazygit

Uninstalling lazygit... (30 files, 86MB)

~/code → git clone https://github.com/jesseduffield/lazygit.git

Cloning into 'lazygit'...
remote: Enumerating objects: 1, done.
remote: Counting objects: 100% (1/1), done.
remote: Total 7887 (delta 0), reused 0 (delta 0), pack-reused 7886
Receiving objects: 100% (7887/7887), 9.45 MiB | 3.68 MiB/s, done.
Resolving deltas: 100% (4629/4629), done.

~/code → cd lazygit

~/code/lazygit → gigit checkout feature/configurable-diffs

Branch 'feature/configurable-diffs' set up to track remote branch 'feature/configurable-diffs' from 'origin'.
Switched to a new branch 'feature/configurable-diffs'

~/code/lazygit → go build

main.go:10:2: cannot find package "github.com/jesseduffield/lazygit/pkg/app" in any of:
        /usr/local/Cellar/go/1.11.4/libexec/src/github.com/jesseduffield/lazygit/pkg/app (from $GOROOT)
        /Users/steven/go/src/github.com/jesseduffield/lazygit/pkg/app (from $GOPATH)
main.go:11:2: cannot find package "github.com/jesseduffield/lazygit/pkg/config" in any of:
        /usr/local/Cellar/go/1.11.4/libexec/src/github.com/jesseduffield/lazygit/pkg/config (from $GOROOT)
        /Users/steven/go/src/github.com/jesseduffield/lazygit/pkg/config (from $GOPATH)

~/code/lazygit → edit

I don't have a background with Go so I'm not sure what do next.

@snoblenet
Copy link

@jesseduffield
Copy link
Owner Author

hey @snoblenet apologies for the late reply I had a holiday and have since been very busy :)

I've just merged master into this PR and it looks pretty much good to go, except for one bug where the currently selected line in staging-lines mode doesn't get highlighted. I've managed to fix this in my rebasing PR that I expect to merge soon but I can't recall how I did it haha.

My plan is to merge that rebasing PR and then merge this in straight afterwards, hopefully soon!

Regarding your go build failure, I believe you need to install dependencies with dep ensure, which is equivalent to npm install or bundle install

For some reason, after merging master into this branch, attempting to open a file caused a crash.
The OS section of the default config in app_config.go didn't get overridden by the user's config, and I'm not sure why.
I'm going to comment it out for now and see if the issue resurfaces in some other way later
@codecov-io
Copy link

Codecov Report

Merging #201 into master will increase coverage by 0.02%.
The diff coverage is 91.3%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #201      +/-   ##
==========================================
+ Coverage    83.5%   83.52%   +0.02%     
==========================================
  Files          19       19              
  Lines        3158     3174      +16     
==========================================
+ Hits         2637     2651      +14     
- Misses        490      491       +1     
- Partials       31       32       +1
Impacted Files Coverage Δ
pkg/commands/os.go 69.5% <ø> (ø) ⬆️
pkg/i18n/dutch.go 100% <100%> (ø) ⬆️
pkg/i18n/polish.go 100% <100%> (ø) ⬆️
pkg/i18n/english.go 100% <100%> (ø) ⬆️
pkg/commands/git.go 60.6% <85.71%> (+0.17%) ⬆️

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 63e2ccf...8412f17. Read the comment docs.

@snoblenet
Copy link

It doesn't seem to be reading my config.yml at all.

In development.log I see: "command":"git diff --color --no-index /dev/null 'foo'".

Is the syntax above correct?

I couldn't find any documentation to answer that question myself sorry.

@jesseduffield
Copy link
Owner Author

When I use your config i.e.:

reporting: 'on'
git:
  fileDiffTemplate: 'git difftool --no-prompt --extcmd="icdiff --cols={{width}}" {{args}} -- {{filename}}'

I get the correct split diff view.

Perhaps your config file is in the wrong location. If you go into lazygit and go up to the status panel at the top, and hit 'o' it should open your config file in your editor (or hit 'e' to directly edit it).

Do you get the expected file when you do that?

@snoblenet
Copy link

It opens to the correct file when I press o but not e.

My config file as it $HOME/dotfiles/lazygit/config.yml.

Previously I had a symbolic link from somewhere in the lazygit dir (I think?) pointing this file but that seems to have disappeared, perhaps during a software update.

Where should I create my link?

I previously have installed Lazygit using Go but have uninstalled that version in favour of Homebrew, which installed it to /usr/local/Cellar/lazygit/0.7.2.

@jesseduffield
Copy link
Owner Author

my config is at /Users/MyAccount/Library/Application Support/jesseduffield/lazygit/config.yml. We use this package to ensure the config file is placed according to the XDG spec:

https://github.com/shibukawa/configdir

@snoblenet
Copy link

I wonder if that package does not like symlinks?

~/Library/Application Support/jesseduffield → ls -la
total 16
drwxr-xr-x    4 steven  staff   128 Nov 29 11:44 .
drwx------  128 steven  staff  4096 Feb 28 11:36 ..
-rw-r--r--@   1 steven  staff  6148 Nov 29 11:44 .DS_Store
lrwxr-xr-x    1 steven  staff    35 Nov 29 11:44 lazygit -> /Users/steven/code/dotfiles/lazygit
~/Library/Application Support/jesseduffield →

@snoblenet
Copy link

Replaced all symlinks with hard files in ~/Library/Application Support/jesseduffield/lazygit and the problem remains. Hmmm...

@mjarkk
Copy link
Contributor

mjarkk commented Mar 11, 2019

Have you tried chmod 666 -R ./* inside ~/Library/Application Support/jesseduffield

@mjarkk
Copy link
Contributor

mjarkk commented Mar 11, 2019

On linux linked folder seems to work fine,

cd ~/.config/jesseduffield/
mv lazygit lazygit-real
ln -s lazygit-real lazygit
cd /to/some/git/repo
lazygit # lazygit opens with my config

@jesseduffield
Copy link
Owner Author

I am regrettably closing this because the codebase has shifted so much since I first put it up and I'd need to come up with a smarter way of supporting it.

@jesseduffield jesseduffield deleted the feature/configurable-diffs branch March 20, 2023 02:36
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.

None yet

4 participants