fac icon indicating copy to clipboard operation
fac copied to clipboard

What about `e` to edit or `o` to open

Open stoivo opened this issue 7 years ago • 15 comments

What about e to edit or o to open .

In case I manually want to merge the conflict

stoivo avatar Feb 12 '18 15:02 stoivo

@stoivo Great idea!

mkchoi212 avatar Feb 13 '18 06:02 mkchoi212

This has been implemented with https://github.com/mkchoi212/fac/commit/afe83276261947c0502675880c1c85b662749031!

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

mkchoi212 avatar May 21 '18 17:05 mkchoi212

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

stoivo avatar May 22 '18 09:05 stoivo

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

mkchoi212 avatar May 22 '18 12:05 mkchoi212

@stoivo The bug has been fixed with https://github.com/mkchoi212/fac/commit/d93d37e9ccac9ce551e3fd9452bb414d6ffe4e3c, I think 🤔 Could you give it a swirl?

mkchoi212 avatar May 23 '18 01:05 mkchoi212

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

stoivo avatar May 24 '18 06:05 stoivo

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

mkchoi212 avatar May 24 '18 19:05 mkchoi212

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

stoivo avatar May 25 '18 05:05 stoivo

Not sure if this is the most elegant solution but https://github.com/mkchoi212/fac/commit/9c16b8a82794095c288c0f2c85b8d741facf2f34 fixes the issue! Let me know if it works for ya :D

mkchoi212 avatar May 25 '18 21:05 mkchoi212

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!

stoivo avatar May 25 '18 22:05 stoivo

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?

mkchoi212 avatar May 25 '18 22:05 mkchoi212

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

stoivo avatar May 27 '18 07:05 stoivo

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?

mkchoi212 avatar May 27 '18 16:05 mkchoi212

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)

stoivo avatar May 28 '18 06:05 stoivo

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 https://github.com/golang/go/issues/4896

mkchoi212 avatar May 28 '18 13:05 mkchoi212