X Tutup
Skip to content

Commit 8469441

Browse files
bchadwicbchadwic
committed
new functionality: current folder './', parent folder '../', absolute 'filename'
1 parent b3a24d2 commit 8469441

File tree

3 files changed

+80
-90
lines changed

3 files changed

+80
-90
lines changed

‎git/git.go‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -351,20 +351,19 @@ func ToplevelDir() (string, error) {
351351

352352
}
353353

354-
func PathFromRepoRoot() (string, error) {
354+
func PathFromRepoRoot() string {
355355
showCmd, err := GitCommand("rev-parse", "--show-prefix")
356356
if err != nil {
357-
return "", err
357+
return ""
358358
}
359359
output, err := run.PrepareCmd(showCmd).Output()
360360
if err != nil {
361-
return "", err
361+
return ""
362362
}
363363
if path := firstLine(output); path != "" {
364-
return path[:len(path)-1], nil
364+
return path[:len(path)-1]
365365
}
366-
return "", nil
367-
366+
return ""
368367
}
369368

370369
func outputLines(output []byte) []string {

‎pkg/cmd/browse/browse.go‎

Lines changed: 22 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@ package browse
33
import (
44
"fmt"
55
"net/http"
6+
"os"
7+
"path/filepath"
8+
"regexp"
69
"strconv"
710
"strings"
811

@@ -136,15 +139,6 @@ func runBrowse(opts *BrowseOptions) error {
136139
if isNumber(opts.SelectorArg) {
137140
url += "/issues/" + opts.SelectorArg
138141
} else {
139-
fileArg, err := parseFileArg(opts.SelectorArg)
140-
if err != nil {
141-
return err
142-
}
143-
path, err := git.PathFromRepoRoot()
144-
if err != nil {
145-
return err
146-
}
147-
fileArg = getRelativePath(path, fileArg)
148142
if opts.Branch != "" {
149143
url += "/tree/" + opts.Branch + "/"
150144
} else {
@@ -155,7 +149,12 @@ func runBrowse(opts *BrowseOptions) error {
155149
}
156150
url += "/tree/" + branchName + "/"
157151
}
158-
url += fileArg
152+
fileArg, err := parseFileArg(opts.SelectorArg)
153+
if err != nil {
154+
return err
155+
}
156+
path := parsePathFromFileArg(fileArg)
157+
url += path
159158
}
160159
}
161160

@@ -189,22 +188,19 @@ func isNumber(arg string) bool {
189188
return err == nil
190189
}
191190

192-
func getRelativePath(path, fileArg string) string {
193-
if !strings.HasPrefix(fileArg, "../") && !strings.HasPrefix(fileArg, "..\\") {
194-
if fileArg == "" {
195-
return path
196-
}
197-
if path == "" {
198-
return fileArg
199-
}
200-
return path + "/" + fileArg
191+
func parsePathFromFileArg(fileArg string) string {
192+
if !hasRelativePrefix(fileArg) {
193+
return fileArg
201194
}
202-
203-
if i := strings.LastIndex(path, "/"); i > 0 {
204-
path = path[:i]
205-
} else {
206-
path = ""
195+
path := filepath.Join(git.PathFromRepoRoot(), fileArg)
196+
match, _ := regexp.Match("(^\\.$)|(^\\.\\./)", []byte(path))
197+
if match {
198+
return ""
207199
}
208-
// recursively remove leading ../ or ..\
209-
return getRelativePath(path, fileArg[3:])
200+
return path
201+
}
202+
203+
func hasRelativePrefix(fileArg string) bool {
204+
return strings.HasPrefix(fileArg, ".."+string(os.PathSeparator)) ||
205+
strings.HasPrefix(fileArg, "."+string(os.PathSeparator))
210206
}

‎pkg/cmd/browse/browse_test.go‎

Lines changed: 53 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ func Test_runBrowse(t *testing.T) {
179179
opts: BrowseOptions{SelectorArg: "path/to/file.txt"},
180180
baseRepo: ghrepo.New("ken", "mrprofessor"),
181181
defaultBranch: "main",
182-
expectedURL: "https://github.com/ken/mrprofessor/tree/main/pkg/cmd/browse/path/to/file.txt",
182+
expectedURL: "https://github.com/ken/mrprofessor/tree/main/path/to/file.txt",
183183
},
184184
{
185185
name: "issue argument",
@@ -204,7 +204,7 @@ func Test_runBrowse(t *testing.T) {
204204
SelectorArg: "main.go",
205205
},
206206
baseRepo: ghrepo.New("bchadwic", "LedZeppelinIV"),
207-
expectedURL: "https://github.com/bchadwic/LedZeppelinIV/tree/trunk/pkg/cmd/browse/main.go",
207+
expectedURL: "https://github.com/bchadwic/LedZeppelinIV/tree/trunk/main.go",
208208
},
209209
{
210210
name: "file with line number",
@@ -213,7 +213,7 @@ func Test_runBrowse(t *testing.T) {
213213
},
214214
baseRepo: ghrepo.New("ravocean", "angur"),
215215
defaultBranch: "trunk",
216-
expectedURL: "https://github.com/ravocean/angur/tree/trunk/pkg/cmd/browse/path/to/file.txt#L32",
216+
expectedURL: "https://github.com/ravocean/angur/tree/trunk/path/to/file.txt#L32",
217217
},
218218
{
219219
name: "file with invalid line number",
@@ -241,7 +241,7 @@ func Test_runBrowse(t *testing.T) {
241241
},
242242
baseRepo: ghrepo.New("github", "ThankYouGitHub"),
243243
wantsErr: false,
244-
expectedURL: "https://github.com/github/ThankYouGitHub/tree/first-browse-pull/pkg/cmd/browse/browse.go#L32",
244+
expectedURL: "https://github.com/github/ThankYouGitHub/tree/first-browse-pull/browse.go#L32",
245245
},
246246
{
247247
name: "no browser with branch file and line number",
@@ -252,18 +252,28 @@ func Test_runBrowse(t *testing.T) {
252252
},
253253
baseRepo: ghrepo.New("mislav", "will_paginate"),
254254
wantsErr: false,
255-
expectedURL: "https://github.com/mislav/will_paginate/tree/3-0-stable/pkg/cmd/browse/init.rb#L6",
255+
expectedURL: "https://github.com/mislav/will_paginate/tree/3-0-stable/init.rb#L6",
256256
},
257257
{
258258
name: "relative path from browse_test.go",
259259
opts: BrowseOptions{
260-
SelectorArg: "browse_test.go",
260+
SelectorArg: "./browse_test.go",
261261
},
262262
baseRepo: ghrepo.New("bchadwic", "gh-graph"),
263263
defaultBranch: "trunk",
264264
expectedURL: "https://github.com/bchadwic/gh-graph/tree/trunk/pkg/cmd/browse/browse_test.go",
265265
wantsErr: false,
266266
},
267+
{
268+
name: "relative path to file in parent folder from browse_test.go",
269+
opts: BrowseOptions{
270+
SelectorArg: "../pr",
271+
},
272+
baseRepo: ghrepo.New("bchadwic", "gh-graph"),
273+
defaultBranch: "trunk",
274+
expectedURL: "https://github.com/bchadwic/gh-graph/tree/trunk/pkg/cmd/pr",
275+
wantsErr: false,
276+
},
267277
}
268278

269279
for _, tt := range tests {
@@ -345,77 +355,62 @@ func Test_parseFileArg(t *testing.T) {
345355
}
346356
}
347357

348-
func Test_getRelativePath(t *testing.T) {
358+
func Test_parsePathFromFileArg(t *testing.T) {
359+
360+
// tests assume path is pkg/cmd/browse
349361
tests := []struct {
350-
name string
351-
path string
352-
fileArg string
353-
expectedPath string
354-
expectedError bool
362+
name string
363+
fileArg string
364+
expectedPath string
355365
}{
356366
{
357-
name: "file in current folder",
358-
path: "cmd/gh",
359-
fileArg: "main.go",
360-
expectedPath: "cmd/gh/main.go",
361-
expectedError: false,
367+
name: "go to parent folder",
368+
fileArg: "../",
369+
expectedPath: "pkg/cmd",
370+
},
371+
{
372+
name: "file in current folder",
373+
fileArg: "./browse.go",
374+
expectedPath: "pkg/cmd/browse/browse.go",
362375
},
363376
{
364-
name: "invalid file in current folder",
365-
path: "cmd/gh",
366-
fileArg: "main.go",
367-
expectedPath: "cmd/gh/main.go/hello",
368-
expectedError: true,
377+
name: "file within parent folder",
378+
fileArg: "../browse.go",
379+
expectedPath: "pkg/cmd/browse.go",
369380
},
370381
{
371-
name: "folder in parent folder",
372-
path: "cmd/gh",
373-
fileArg: "../gen-docs/main.go",
374-
expectedPath: "cmd/gen-docs/main.go",
375-
expectedError: false,
382+
name: "file within parent folder uncleaned",
383+
fileArg: ".././//browse.go",
384+
expectedPath: "pkg/cmd/browse.go",
376385
},
377386
{
378-
name: "folder in several folders up",
379-
path: "/pkg/cmd/browse",
380-
fileArg: "../../../api/cache.go",
381-
expectedPath: "api/cache.go",
382-
expectedError: false,
387+
name: "different path from root directory",
388+
fileArg: "../../../internal/build/build.go",
389+
expectedPath: "internal/build/build.go",
383390
},
384391
{
385-
name: "going to root of repository",
386-
path: "/pkg/cmd/browse",
387-
fileArg: "../../../",
388-
expectedPath: "",
389-
expectedError: false,
392+
name: "folder in root folder",
393+
fileArg: "pkg",
394+
expectedPath: "pkg",
390395
},
391396
{
392-
name: "trying to go past root of repository",
393-
path: "/pkg/cmd/browse",
394-
fileArg: "../../../../../../../../",
395-
expectedPath: "",
396-
expectedError: false,
397+
name: "subfolder in root folder",
398+
fileArg: "pkg/cmd",
399+
expectedPath: "pkg/cmd",
397400
},
398401
{
399-
name: "windows users",
400-
path: "/pkg/cmd/browse",
401-
fileArg: "..\\",
402-
expectedPath: "/pkg/cmd",
403-
expectedError: false,
402+
name: "go out of repository",
403+
fileArg: "../../../../../../",
404+
expectedPath: "",
404405
},
405406
{
406-
name: "possible combination users",
407-
path: "/pkg/cmd/pr/checkout",
408-
fileArg: "..\\../..\\",
409-
expectedPath: "/pkg",
410-
expectedError: false,
407+
name: "go to root of repository",
408+
fileArg: "../../../",
409+
expectedPath: "",
411410
},
412411
}
413412
for _, tt := range tests {
414-
path := getRelativePath(tt.path, tt.fileArg)
415-
if tt.expectedError {
416-
assert.NotEqual(t, tt.expectedPath, path)
417-
} else {
418-
assert.Equal(t, tt.expectedPath, path)
419-
}
413+
path := parsePathFromFileArg(tt.fileArg)
414+
assert.Equal(t, tt.expectedPath, path)
420415
}
421416
}

0 commit comments

Comments
 (0)
X Tutup