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

Escape paths #32

Merged
merged 11 commits into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 36 additions & 32 deletions cmd/pathctl/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,75 +7,61 @@ import (
"os"
"strings"

"al.essio.dev/pkg/tools/dirlist"
"al.essio.dev/pkg/tools/internal/version"
"al.essio.dev/pkg/tools/pathlist"
)

const (
programme = "pathctl"
program = "pathctl"
)

var (
helpMode bool
versionMode bool
listMode bool
noprefixMode bool
noPrefixMode bool
dropMode bool
)

var (
envVar string
)

var cmdHandlers map[string]func(d pathlist.List)
var cmdHandlers map[string]func(d dirlist.List)
alessio marked this conversation as resolved.
Show resolved Hide resolved

func init() {
flag.BoolVar(&helpMode, "help", false, "display this help and exit.")
flag.BoolVar(&versionMode, "version", false, "output version information and exit.")
flag.BoolVar(&dropMode, "D", false, "drop the path before adding it again to the list.")
flag.BoolVar(&noprefixMode, "noprefix", false, "output the variable contents only.")
flag.BoolVar(&noPrefixMode, "noprefix", false, "output the variable contents only.")
flag.BoolVar(&listMode, "L", false, "use a newline character as path list separator.")
flag.StringVar(&envVar, "E", "PATH", "input environment variable.")
flag.Usage = usage
flag.CommandLine.SetOutput(os.Stderr)

cmdHandlers = func() map[string]func(pathlist.List) {
hAppend := func(d pathlist.List) {
if dropMode {
d.Drop(flag.Arg(1))
}
d.Append(flag.Arg(1))
}
hDrop := func(d pathlist.List) { d.Drop(flag.Arg(1)) }
hPrepend := func(d pathlist.List) {
if dropMode {
d.Drop(flag.Arg(1))
}
d.Prepend(flag.Arg(1))
}

return map[string]func(pathlist.List){
"append": hAppend,
"drop": hDrop,
"prepend": hPrepend,
cmdHandlers = func() map[string]func(dirlist.List) {
return map[string]func(dirlist.List){
"append": cmdHandlerAppend,
"drop": cmdHandlerDrop,
"prepend": cmdHandlerPrepend,

// aliases
"a": hAppend,
"d": hDrop,
"p": hPrepend,
"a": cmdHandlerAppend,
"d": cmdHandlerDrop,
"p": cmdHandlerPrepend,
}
}()
}

func main() {
log.SetFlags(0)
log.SetPrefix(fmt.Sprintf("%s: ", programme))
log.SetPrefix(fmt.Sprintf("%s: ", program))
log.SetOutput(os.Stderr)
flag.Parse()

handleHelpAndVersionModes()

dirs := pathlist.New()
dirs := dirlist.New()
dirs.LoadEnv(envVar)

if flag.NArg() < 1 {
Comment on lines 61 to 67
Copy link
Contributor

Choose a reason for hiding this comment

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

📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [78-94]

The main function uses the dirlist package to load and manipulate environment variables but does not handle potential errors.

Implement error handling for the LoadEnv and other dirlist operations to ensure robustness.

Expand All @@ -91,11 +77,11 @@ func main() {
}
}

func printPathList(d pathlist.List) {
func printPathList(d dirlist.List) {
var sb = strings.Builder{}
sb.Reset()

printPrefix := !noprefixMode
printPrefix := !noPrefixMode

switch {
case listMode:
Expand Down Expand Up @@ -138,7 +124,7 @@ Commands:
prepend, p prepend a path to the list.

Options:
`, programme)
`, program)
_, _ = fmt.Fprintln(os.Stderr, s)

flag.PrintDefaults()
Expand All @@ -152,3 +138,21 @@ element of the path list.
If COMMAND is not provided, it prints the contents of the PATH
environment variable.`)
}

func cmdHandlerAppend(d dirlist.List) {
if dropMode {
d.Drop(flag.Arg(1))
}
d.Append(flag.Arg(1))
}

func cmdHandlerDrop(d dirlist.List) {
d.Drop(flag.Arg(1))
}

func cmdHandlerPrepend(d dirlist.List) {
if dropMode {
d.Drop(flag.Arg(1))
}
d.Prepend(flag.Arg(1))
}
77 changes: 41 additions & 36 deletions pathlist/list.go → dirlist/list.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
// Package pathlist implements functions to manipulate PATH-like
// Package dirlist implements functions to manipulate PATH-like
// environment variables.
package pathlist
package dirlist

import (
"fmt"
"os"
"path/filepath"
"slices"
Expand All @@ -19,9 +20,6 @@
// Contains returns true if the list contains the path.
Contains(string) bool

// Nil returns true if the list is emppty.
Nil() bool

// Load reads the list of directories from a string.
Load(string)

Expand Down Expand Up @@ -60,17 +58,13 @@
}

func (d *dirList) Contains(p string) bool {
return slices.Contains(d.lst, p)
return slices.Contains(d.lst, filepath.Clean(p))
}

func (d *dirList) Reset() {
d.init()
}

func (d *dirList) Nil() bool {
return d.lst == nil || len(d.lst) == 0
}

func (d *dirList) Load(s string) {
d.src = s
d.load()
Expand All @@ -80,26 +74,25 @@
d.Load(os.Getenv(s))
}

func (d *dirList) Slice() []string {
if d.Nil() {
return []string{}
func (d *dirList) Slice() (dst []string) {
if len(d.lst) == 0 {
return
}

dst := make([]string, len(d.lst))
n := copy(dst, d.lst)
if n != len(d.lst) {
panic("couldn't copy the list")
dst = make([]string, len(d.lst))
if n := copy(dst, d.lst); n == len(d.lst) {
return dst
}

return dst
panic("couldn't copy the list")

Check warning on line 87 in dirlist/list.go

View check run for this annotation

Codecov / codecov/patch

dirlist/list.go#L87

Added line #L87 was not covered by tests
alessio marked this conversation as resolved.
Show resolved Hide resolved
}

func (d *dirList) String() string {
if !d.Nil() {
return strings.Join(d.lst, string(filepath.ListSeparator))
if len(d.lst) == 0 {
return ""
}

return ""
return strings.Join(d.lst, string(filepath.ListSeparator))
}

func (d *dirList) load() {
Expand All @@ -108,7 +101,7 @@

func (d *dirList) Append(path string) {
p := filepath.Clean(path)
if d.Nil() {
if len(d.lst) == 0 {
d.lst = []string{p}
return
}
Expand All @@ -119,9 +112,10 @@
}

func (d *dirList) Drop(path string) {
if d.Nil() {
if len(d.lst) == 0 {
return
}

p := filepath.Clean(path)

if idx := slices.Index(d.lst, p); idx != -1 {
Expand All @@ -131,7 +125,7 @@

func (d *dirList) Prepend(path string) {
p := filepath.Clean(path)
if d.Nil() {
if len(d.lst) == 0 {
d.lst = []string{p}
return
}
Expand All @@ -147,12 +141,16 @@
}

func (d *dirList) cleanPathVar() []string {
if d.src == "" {
return cleanPathVar(d.src)
}

func cleanPathVar(src string) []string {
if src == "" {
return nil
}

pthSlice := filepath.SplitList(d.src)
if pthSlice == nil {
pthSlice := filepath.SplitList(src)
if len(pthSlice) == 0 {
return nil
}

Expand All @@ -161,22 +159,28 @@

func (d *dirList) clone(o *dirList) *dirList {
o.src = d.src
o.lst = make([]string, len(d.lst))
copy(o.lst, d.lst)

n := len(d.lst)
o.lst = make([]string, n)

if m := copy(o.lst, d.lst); n != m {
panic(fmt.Sprintf("copy: expected %d items, got %d", n, m))

Check warning on line 167 in dirlist/list.go

View check run for this annotation

Codecov / codecov/patch

dirlist/list.go#L167

Added line #L167 was not covered by tests
}

return o
}

func removeDups[T comparable](col []T, applyFn func(T) (T, bool)) []T {
var uniq = make([]T, 0)
ks := make(map[T]interface{})
func removeDups(col []string, applyFn func(string) (string, bool)) []string {
var uniq = make([]string, 0)
ks := make(map[string]interface{})

for _, el := range col {
vv, ok := applyFn(el)
alessio marked this conversation as resolved.
Show resolved Hide resolved
if !ok {
continue
}

vv = filepath.Join(filepath.Split(vv))
if _, ok := ks[vv]; !ok {
uniq = append(uniq, vv)
ks[vv] = struct{}{}
Expand All @@ -187,10 +191,11 @@
}

var filterEmptyStrings = func(s string) (string, bool) {
clean := filepath.Clean(s)
if clean != "" {
return clean, true
if strings.TrimSpace(s) == "" {
return s, false
}

return clean, false
// It'd pointless to check filepath.Clean()'s return
// value's nil-ness as it would never be "".
return filepath.Clean(s), true
}
36 changes: 36 additions & 0 deletions dirlist/list_internal_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package dirlist

import (
"testing"

"github.com/stretchr/testify/require"
)

func Test_newDirList(t *testing.T) {
d := new(dirList)
d.Reset()
require.NotNil(t, d)
require.Equal(t, "", d.String())

d.Append("/sbin")
require.Equal(t, "/sbin", d.String())

d1 := new(dirList)
d1.Reset()
d1 = d.clone(d1)
d.Append("/var")
require.NotEqual(t, &d, &d1)
d1.Prepend("/usr/bin")
d1.Append("/usr/local/bin")
require.Equal(t, "/usr/bin:/sbin:/usr/local/bin", d1.String())
}

func Test_removeDups(t *testing.T) {
require.Equal(t,
[]string{"alpha", "bravo", "charlie", "."},
removeDups(
[]string{"alpha", "bravo", "charlie", "bravo", " ", "."},
filterEmptyStrings,
),
)
}
Loading
Loading