Skip to content

Commit 8219710

Browse files
samcoesamcoe
committed
Address PR comments
1 parent 0e83805 commit 8219710

File tree

2 files changed

+117
-141
lines changed

2 files changed

+117
-141
lines changed

‎pkg/cmd/repo/sync/sync.go‎

Lines changed: 108 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -6,28 +6,25 @@ import (
66
"net/http"
77
"regexp"
88

9-
"github.com/AlecAivazis/survey/v2"
109
"github.com/MakeNowJust/heredoc"
1110
"github.com/cli/cli/api"
1211
"github.com/cli/cli/context"
1312
"github.com/cli/cli/internal/ghrepo"
1413
"github.com/cli/cli/pkg/cmdutil"
1514
"github.com/cli/cli/pkg/iostreams"
16-
"github.com/cli/cli/pkg/prompt"
1715
"github.com/spf13/cobra"
1816
)
1917

2018
type SyncOptions struct {
21-
HttpClient func() (*http.Client, error)
22-
IO *iostreams.IOStreams
23-
BaseRepo func() (ghrepo.Interface, error)
24-
Remotes func() (context.Remotes, error)
25-
Git gitClient
26-
DestArg string
27-
SrcArg string
28-
Branch string
29-
Force bool
30-
SkipConfirm bool
19+
HttpClient func() (*http.Client, error)
20+
IO *iostreams.IOStreams
21+
BaseRepo func() (ghrepo.Interface, error)
22+
Remotes func() (context.Remotes, error)
23+
Git gitClient
24+
DestArg string
25+
SrcArg string
26+
Branch string
27+
Force bool
3128
}
3229

3330
func NewCmdSync(f *cmdutil.Factory, runF func(*SyncOptions) error) *cobra.Command {
@@ -42,13 +39,21 @@ func NewCmdSync(f *cmdutil.Factory, runF func(*SyncOptions) error) *cobra.Comman
4239
cmd := &cobra.Command{
4340
Use: "sync [<destination-repository>]",
4441
Short: "Sync a repository",
45-
Long: heredoc.Doc(`
46-
Sync destination repository from source repository.
42+
Long: heredoc.Docf(`
43+
Sync destination repository from source repository. Syncing will take a branch
44+
on the source repository and merge it into the branch of the same name on the
45+
destination repository. A fast forward merge will be used execept when the
46+
%[1]s--force%[1]s flag is specified, then the two branches will by synced using
47+
a hard reset.
4748
4849
Without an argument, the local repository is selected as the destination repository.
49-
By default the source repository is the parent of the destination repository.
50-
The source repository can be overridden with the --source flag.
51-
`),
50+
51+
By default the source repository is the parent of the destination repository,
52+
this can be overridden with the %[1]s--source%[1]s flag.
53+
54+
The source repository default branch is selected automatically, but can be
55+
overridden with the %[1]s--branch%[1]s flag.
56+
`, "`"),
5257
Example: heredoc.Doc(`
5358
# Sync local repository from remote parent
5459
$ gh repo sync
@@ -67,9 +72,6 @@ func NewCmdSync(f *cmdutil.Factory, runF func(*SyncOptions) error) *cobra.Comman
6772
if len(args) > 0 {
6873
opts.DestArg = args[0]
6974
}
70-
if !opts.IO.CanPrompt() && !opts.SkipConfirm {
71-
return &cmdutil.FlagError{Err: errors.New("`--confirm` required when not running interactively")}
72-
}
7375
if runF != nil {
7476
return runF(&opts)
7577
}
@@ -80,46 +82,91 @@ func NewCmdSync(f *cmdutil.Factory, runF func(*SyncOptions) error) *cobra.Comman
8082
cmd.Flags().StringVarP(&opts.SrcArg, "source", "s", "", "Source repository")
8183
cmd.Flags().StringVarP(&opts.Branch, "branch", "b", "", "Branch to sync")
8284
cmd.Flags().BoolVarP(&opts.Force, "force", "", false, "Discard destination repository changes")
83-
cmd.Flags().BoolVarP(&opts.SkipConfirm, "confirm", "y", false, "Skip the confirmation prompt")
8485
return cmd
8586
}
8687

8788
func syncRun(opts *SyncOptions) error {
88-
httpClient, err := opts.HttpClient()
89+
if opts.DestArg == "" {
90+
return syncLocalRepo(opts)
91+
} else {
92+
return syncRemoteRepo(opts)
93+
}
94+
}
95+
96+
func syncLocalRepo(opts *SyncOptions) error {
97+
var err error
98+
var srcRepo ghrepo.Interface
99+
100+
if opts.SrcArg != "" {
101+
srcRepo, err = ghrepo.FromFullName(opts.SrcArg)
102+
} else {
103+
srcRepo, err = opts.BaseRepo()
104+
}
89105
if err != nil {
90106
return err
91107
}
92-
apiClient := api.NewClientFromHTTP(httpClient)
93108

94-
var local bool
95-
var destRepo, srcRepo ghrepo.Interface
96-
97-
if opts.DestArg == "" {
98-
local = true
99-
destRepo, err = opts.BaseRepo()
109+
if opts.Branch == "" {
110+
httpClient, err := opts.HttpClient()
100111
if err != nil {
101112
return err
102113
}
103-
} else {
104-
destRepo, err = ghrepo.FromFullName(opts.DestArg)
114+
apiClient := api.NewClientFromHTTP(httpClient)
115+
opts.IO.StartProgressIndicator()
116+
opts.Branch, err = api.RepoDefaultBranch(apiClient, srcRepo)
117+
opts.IO.StopProgressIndicator()
105118
if err != nil {
106119
return err
107120
}
108121
}
109122

123+
opts.IO.StartProgressIndicator()
124+
err = executeLocalRepoSync(srcRepo, opts)
125+
opts.IO.StopProgressIndicator()
126+
if err != nil {
127+
if errors.Is(err, divergingError) {
128+
return fmt.Errorf("can't sync because there are diverging changes, you can use `--force` to overwrite the changes")
129+
}
130+
if errors.Is(err, dirtyRepoError) {
131+
return fmt.Errorf("can't sync because there are local changes, please commit or stash them")
132+
}
133+
return err
134+
}
135+
136+
if opts.IO.IsStdoutTTY() {
137+
cs := opts.IO.ColorScheme()
138+
srcStr := fmt.Sprintf("%s:%s", ghrepo.FullName(srcRepo), opts.Branch)
139+
destStr := fmt.Sprintf(".:%s", opts.Branch)
140+
success := fmt.Sprintf("Synced %s from %s\n", cs.Bold(destStr), cs.Bold(srcStr))
141+
fmt.Fprintf(opts.IO.Out, "%s %s", cs.SuccessIcon(), success)
142+
}
143+
144+
return nil
145+
}
146+
147+
func syncRemoteRepo(opts *SyncOptions) error {
148+
httpClient, err := opts.HttpClient()
149+
if err != nil {
150+
return err
151+
}
152+
apiClient := api.NewClientFromHTTP(httpClient)
153+
154+
var destRepo, srcRepo ghrepo.Interface
155+
156+
destRepo, err = ghrepo.FromFullName(opts.DestArg)
157+
if err != nil {
158+
return err
159+
}
160+
110161
if opts.SrcArg == "" {
111-
if local {
112-
srcRepo = destRepo
113-
} else {
114-
opts.IO.StartProgressIndicator()
115-
srcRepo, err = api.RepoParent(apiClient, destRepo)
116-
opts.IO.StopProgressIndicator()
117-
if err != nil {
118-
return err
119-
}
120-
if srcRepo == nil {
121-
return fmt.Errorf("can't determine source repo for %s because repo is not fork", ghrepo.FullName(destRepo))
122-
}
162+
opts.IO.StartProgressIndicator()
163+
srcRepo, err = api.RepoParent(apiClient, destRepo)
164+
opts.IO.StopProgressIndicator()
165+
if err != nil {
166+
return err
167+
}
168+
if srcRepo == nil {
169+
return fmt.Errorf("can't determine source repo for %s because repo is not fork", ghrepo.FullName(destRepo))
123170
}
124171
} else {
125172
srcRepo, err = ghrepo.FromFullName(opts.SrcArg)
@@ -128,7 +175,7 @@ func syncRun(opts *SyncOptions) error {
128175
}
129176
}
130177

131-
if !local && destRepo.RepoHost() != srcRepo.RepoHost() {
178+
if destRepo.RepoHost() != srcRepo.RepoHost() {
132179
return fmt.Errorf("can't sync repos from different hosts")
133180
}
134181

@@ -141,52 +188,31 @@ func syncRun(opts *SyncOptions) error {
141188
}
142189
}
143190

144-
srcStr := fmt.Sprintf("%s:%s", ghrepo.FullName(srcRepo), opts.Branch)
145-
destStr := fmt.Sprintf("%s:%s", ghrepo.FullName(destRepo), opts.Branch)
146-
if local {
147-
destStr = fmt.Sprintf(".:%s", opts.Branch)
148-
}
149-
cs := opts.IO.ColorScheme()
150-
if !opts.SkipConfirm && opts.IO.CanPrompt() {
151-
if opts.Force {
152-
fmt.Fprintf(opts.IO.ErrOut, "%s Using --force will cause diverging commits on %s to be discarded\n", cs.WarningIcon(), destStr)
153-
}
154-
var confirmed bool
155-
confirmQuestion := &survey.Confirm{
156-
Message: fmt.Sprintf("Sync %s from %s?", destStr, srcStr),
157-
Default: false,
158-
}
159-
err := prompt.SurveyAskOne(confirmQuestion, &confirmed)
160-
if err != nil {
161-
return err
162-
}
163-
164-
if !confirmed {
165-
return cmdutil.CancelError
166-
}
167-
}
168-
169191
opts.IO.StartProgressIndicator()
170-
if local {
171-
err = syncLocalRepo(srcRepo, opts)
172-
} else {
173-
err = syncRemoteRepo(apiClient, destRepo, srcRepo, opts)
174-
}
192+
err = executeRemoteRepoSync(apiClient, destRepo, srcRepo, opts)
175193
opts.IO.StopProgressIndicator()
176-
177194
if err != nil {
195+
if errors.Is(err, divergingError) {
196+
return fmt.Errorf("can't sync because there are diverging changes, you can use `--force` to overwrite the changes")
197+
}
178198
return err
179199
}
180200

181201
if opts.IO.IsStdoutTTY() {
182-
success := cs.Bold(fmt.Sprintf("Synced %s from %s\n", destStr, srcStr))
183-
fmt.Fprintf(opts.IO.Out, "%s %s", cs.SuccessIconWithColor(cs.GreenBold), success)
202+
cs := opts.IO.ColorScheme()
203+
srcStr := fmt.Sprintf("%s:%s", ghrepo.FullName(srcRepo), opts.Branch)
204+
destStr := fmt.Sprintf("%s:%s", ghrepo.FullName(destRepo), opts.Branch)
205+
success := fmt.Sprintf("Synced %s from %s\n", cs.Bold(destStr), cs.Bold(srcStr))
206+
fmt.Fprintf(opts.IO.Out, "%s %s", cs.SuccessIcon(), success)
184207
}
185208

186209
return nil
187210
}
188211

189-
func syncLocalRepo(srcRepo ghrepo.Interface, opts *SyncOptions) error {
212+
var divergingError = errors.New("diverging commits")
213+
var dirtyRepoError = errors.New("dirty repo")
214+
215+
func executeLocalRepoSync(srcRepo ghrepo.Interface, opts *SyncOptions) error {
190216
// Remotes precedence by name
191217
// 1. upstream
192218
// 2. github
@@ -213,7 +239,7 @@ func syncLocalRepo(srcRepo ghrepo.Interface, opts *SyncOptions) error {
213239
}
214240

215241
if !fastForward && !opts.Force {
216-
return fmt.Errorf("can't sync because there are diverging commits, you can use `--force` to overwrite the commits on .:%s", branch)
242+
return divergingError
217243
}
218244
}
219245

@@ -222,7 +248,7 @@ func syncLocalRepo(srcRepo ghrepo.Interface, opts *SyncOptions) error {
222248
return err
223249
}
224250
if dirtyRepo {
225-
return fmt.Errorf("can't sync because there are local changes, please commit or stash them then try again")
251+
return dirtyRepoError
226252
}
227253

228254
startBranch, err := git.CurrentBranch()
@@ -258,7 +284,7 @@ func syncLocalRepo(srcRepo ghrepo.Interface, opts *SyncOptions) error {
258284
return nil
259285
}
260286

261-
func syncRemoteRepo(client *api.Client, destRepo, srcRepo ghrepo.Interface, opts *SyncOptions) error {
287+
func executeRemoteRepoSync(client *api.Client, destRepo, srcRepo ghrepo.Interface, opts *SyncOptions) error {
262288
commit, err := latestCommit(client, srcRepo, opts.Branch)
263289
if err != nil {
264290
return err
@@ -270,9 +296,7 @@ func syncRemoteRepo(client *api.Client, destRepo, srcRepo ghrepo.Interface, opts
270296
err = syncFork(client, destRepo, opts.Branch, commit.Object.SHA, opts.Force)
271297
var httpErr api.HTTPError
272298
if err != nil && errors.As(err, &httpErr) && notFastForwardErrorMessage.MatchString(httpErr.Message) {
273-
destStr := fmt.Sprintf("%s:%s", ghrepo.FullName(destRepo), opts.Branch)
274-
return fmt.Errorf("can't sync because there are diverging commits, you can use `--force` to overwrite the commits on %s",
275-
destStr)
299+
return divergingError
276300
}
277301

278302
return err

0 commit comments

Comments
 (0)