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

Draft: Add support for automatic branch tracking #54

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mdedonno1337
Copy link
Contributor

This is a draft related to the issue #50. Only to have a tracked discussion on how to implement this feature.

The idea is to try first to create a new branch with remote tracking; if the branch already exists, it will fail and trigger the second command doing a normal git checkout.

I'm using this in a bash script to do fzf branch managment ( 😄 ) since years and it works fine.

@mdedonno1337 mdedonno1337 force-pushed the feature/automatic_tracking_remote_branch branch from 3604378 to b7d2890 Compare March 12, 2021 14:48
Not working for some already existing branches (like "master").
@mdedonno1337
Copy link
Contributor Author

Some thoughts:

  1. If implemented, how useful will the 'track' option be?
  2. Is it a good idea to stuff the command the s:branch_actions variable, or should we add an option in the s:branch_actions, something like if_fail or alternative_command or something, and move the retry commands near the system() command?
  3. Letting bash do the work is probably easier that making the error management in vimscript (let ret = system(...); if ret == error_code; then ...); what do you think? If so, will it be tested on Windows and Mac?

@mdedonno1337 mdedonno1337 marked this pull request as ready for review March 20, 2021 16:09
@@ -66,6 +66,7 @@ function! fzf_checkout#execute(type, action, lines) abort
let l:branch = trim(shellescape(split(a:lines[2])[0]), l:trimchars)
endif
endif
let l:branchlo = substitute(l:branch, '^origin/', '', 'g')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, this is one of the "lazy" implementation I came a long time ago, but obviously you can have other names for the remote, and branches and remotes can also have / in the name.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the name origin, you are absolutely right, it was just a first draft, to be corrected.

For the slash being possibly part of the branch name, I thought about that, but did not know that the origin name could also contain one...
I may have a solution, but we will not be able to distinguish between first/second/last, with first/second being the remote and last the branch name, against first the remote name and second/last being the branch name.
I hope that no sane developer would use this kind of logic to organize branches and remotes...

@stsewd
Copy link
Owner

stsewd commented Mar 26, 2021

If implemented, how useful will the 'track' option be?

I think if we find a way to make this work for all cases that option could be deprecated, or maybe just leave it around.

Is it a good idea to stuff the command the s:branch_actions variable, or should we add an option in the s:branch_actions, something like if_fail or alternative_command or something, and move the retry commands near the system() command?

I like the idea, maybe accept a list of commands instead of just one, so they are retried one after one.

Letting bash do the work is probably easier that making the error management in vimscript (let ret = system(...); if ret == error_code; then ...); what do you think? If so, will it be tested on Windows and Mac?

Yeah, having this in bash is more easy, but introduces problems with compatibility with mac/windows and even with the shell being used, that's why I'm trying to have more this on vimscript when possible.

@mdedonno1337
Copy link
Contributor Author

If implemented, how useful will the 'track' option be?

I think if we find a way to make this work for all cases that option could be deprecated, or maybe just leave it around.

Is it a good idea to stuff the command the s:branch_actions variable, or should we add an option in the s:branch_actions, something like if_fail or alternative_command or something, and move the retry commands near the system() command?

I like the idea, maybe accept a list of commands instead of just one, so they are retried one after one.

Letting bash do the work is probably easier that making the error management in vimscript (let ret = system(...); if ret == error_code; then ...); what do you think? If so, will it be tested on Windows and Mac?

Yeah, having this in bash is more easy, but introduces problems with compatibility with mac/windows and even with the shell being used, that's why I'm trying to have more this on vimscript when possible.

Good remarks and arguments. Thanks for the feedback!
I will think about it and make a draft I have something intelligent. Will let you know.

@mdedonno1337 mdedonno1337 deleted the feature/automatic_tracking_remote_branch branch March 27, 2021 16:55
@mdedonno1337 mdedonno1337 restored the feature/automatic_tracking_remote_branch branch March 27, 2021 16:56
@mdedonno1337 mdedonno1337 reopened this Mar 27, 2021
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.

2 participants