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

What about e to edit or o to open #30

Open
stoivo opened this issue Feb 12, 2018 · 15 comments
Open

What about e to edit or o to open #30

stoivo opened this issue Feb 12, 2018 · 15 comments
Labels

Comments

@stoivo
Copy link

stoivo commented Feb 12, 2018

What about e to edit or o to open .

In case I manually want to merge the conflict

@mkchoi212
Copy link
Owner

@stoivo Great idea!

@mkchoi212
Copy link
Owner

mkchoi212 commented May 21, 2018

This has been implemented with afe8327!

It hasn't been released yet but I would love to get your feedback on the current implementation!
To get the latest version of fac, please run go get github.com/mkchoi212/fac

@stoivo
Copy link
Author

stoivo commented May 22, 2018

It works when I have editor set to vim, also it works with subl but when I set it to subl -w it doesn't open. And when I click e 3 more times. It blows up

 [w,a,s,d,e,?] >> epanic: runtime error: slice bounds out of range

goroutine 75 [running]:
github.com/nsf/termbox-go.PollEvent(0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
	/Users/simon/go/src/github.com/nsf/termbox-go/api.go:348 +0x8bb
github.com/jroimartin/gocui.(*Gui).MainLoop.func1(0xc4201fe090)
	/Users/simon/go/src/github.com/jroimartin/gocui/gui.go:354 +0x30
created by github.com/jroimartin/gocui.(*Gui).MainLoop
	/Users/simon/go/src/github.com/jroimartin/gocui/gui.go:352 +0x6e

@mkchoi212
Copy link
Owner

@stoivo Hmmm I will look in to it. Thanks for the feedback though 👍

@mkchoi212
Copy link
Owner

@stoivo The bug has been fixed with d93d37e, I think 🤔 Could you give it a swirl?

@stoivo
Copy link
Author

stoivo commented May 24, 2018

Still did work when set to subl -w

$ EDITOR='subl -w' ~/go/bin/fac
fac: exec: "subl -w": executable file not found in $PATH
simon at simon in ~/dev/work/name/repo on master_with_conflict*
$ echo $PATH
/usr/local/opt/imagemagick@6/bin:/Users/name/.rbenv/shims:/usr/local/git/bin:/usr/local/bin:/usr/bin:/usr/local/sbin:Users/simon/.rbenv/shims:/usr/local/bin:/usr/bin
simon at simon in ~/dev/work/fakturabank/repo on master_with_conflict*
$ which subl
/usr/local/bin/subl

@mkchoi212
Copy link
Owner

Oh I didn't realize the code breaks when you have a flag in the EDITOR environment variable. I will fix that real quick 👍

@stoivo
Copy link
Author

stoivo commented May 25, 2018

Great! Comment here when thats done and I will test it :D

@mkchoi212
Copy link
Owner

Not sure if this is the most elegant solution but 9c16b8a fixes the issue! Let me know if it works for ya :D

@stoivo
Copy link
Author

stoivo commented May 25, 2018

Awesome, not it doesn't crash 💥
Two comments

  • But nothing happens when you solve the conflict. I tested with sublime and vim. It is still unresolved when I get back to fac.
  • It would be great if you created a FAC_EDITOR environment variable it would try and fallback to EDITOR if it's not set. its a best practice

And thank you for doing all this for everyone!

@mkchoi212
Copy link
Owner

It would be great if you created a FAC_EDITOR environment variable...

I will definitely look into that 👍

But nothing happens when you solve the conflict...

Hmm I can't re-create this on my side. Can I see your git status?

@stoivo
Copy link
Author

stoivo commented May 27, 2018

Check out this demo.
http://recordit.co/7BfEDqrNcz
Maybe I did't explain myself good

@mkchoi212
Copy link
Owner

mkchoi212 commented May 27, 2018

Ah I see what you did. If you look at https://camo.githubusercontent.com/0178c2f554229485cbfd94e3439b4f27d093d2fe/68747470733a2f2f692e696d6775722e636f6d2f47734a4d5249702e676966, I made it so that the conflict markers must stay intact. If not, the prompt turns red, signing an error.

This was because it would be less work by the user and also help them fix their mistakes by not making their action conclusive. What do you think?

@stoivo
Copy link
Author

stoivo commented May 28, 2018

Clever, but unexpected.
So what we do is not to accept any of the branches but modifying all the branches. When returned to fac I can accept one of the branches.
I fear you will get issues reporting the edit not working. So maybe you can add a note to the help section on the right side, like e - manually edit the code (leave the tree in place)

@mkchoi212
Copy link
Owner

Good point. I'm thinking about putting comments like how git add -p does it when you press e to manually edit code chunk. The only trouble is with Go's ioutil.Tempfile, you can't specify the file name's suffix yet; so no syntax highlighting :p

It loooks like it will be implemented around next month golang/go#4896

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants