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

new pathctl command #18

Merged
merged 1 commit into from
Dec 12, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
28 changes: 21 additions & 7 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,14 @@ ifdef verbose
VERBOSE = -v
endif

all: build check
all: generate build check

BUILD_TARGETS := build install

build: generate
build: BUILD_ARGS=-o $(BUILDDIR)/

$(BUILD_TARGETS): generate $(BUILDDIR)/
$(BUILD_TARGETS): $(BUILDDIR)/
go $@ $(VERBOSE) -mod=readonly $(BUILD_FLAGS) $(BUILD_ARGS) ./...

$(BUILDDIR)/:
Expand Down Expand Up @@ -67,15 +68,28 @@ clean:
rm -rf $(BUILDDIR)
rm -f \
$(COVERAGE_REPORT_FILENAME) \
generate-stamp
generate-stamp version-stamp

version-stamp: generate
cp internal/version/version.txt $@

list:
@echo $(BINS) | tr ' ' '\n'

macos-codesign: build
codesign --verbose -s $(CODESIGN_IDENTITY) --options=runtime ./build/*

unixtools.dmg: macos-codesign
create-dmg --volname unixtools --codesign $(CODESIGN_IDENTITY) --sandbox-safe $@ ./build
codesign --verbose -s $(CODESIGN_IDENTITY) --options=runtime $(BUILDDIR)/*

unixtools.pkg: version-stamp macos-codesign
pkgbuild --identifier io.asscrypto.unixtools \
--install-location ./Library/ --root $(BUILDDIR) $@

unixtools.dmg: version-stamp macos-codesign
VERSION=$(shell cat version-stamp); \
mkdir -p dist/unixtools-$${VERSION}/bin ; \
cp -a $(BUILDDIR)/* dist/unixtools-$${VERSION}/bin/ ; \
chmod 0755 dist/unixtools-$${VERSION}/bin/* ; \
create-dmg --volname unixtools --codesign $(CODESIGN_IDENTITY) \
--sandbox-safe --no-internet-enable \
$@ dist/unixtools-$${VERSION}

.PHONY: all clean check distclean build list macos-codesign generate
146 changes: 146 additions & 0 deletions cmd/pathctl/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
package main

import (
"flag"
"fmt"
"log"
"os"
"path/filepath"

"github.com/alessio/unixtools/internal/path"
"github.com/alessio/unixtools/internal/version"
)

const (
progName = "pathctl"
)

var (
helpMode bool
versionMode bool
pathListSep string
)

var (
envVar string
paths path.List
)

func init() {
flag.BoolVar(&helpMode, "help", false, "display this help and exit.")
flag.BoolVar(&versionMode, "version", false, "output version information and exit.")
flag.StringVar(&pathListSep, "s", string(os.PathListSeparator), "path list separator.")
flag.StringVar(&envVar, "e", "PATH", "input environment variable")
flag.Usage = usage
flag.CommandLine.SetOutput(os.Stderr)
}

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

handleHelpAndVersionModes()

paths = path.NewPathList(envVar)

if flag.NArg() < 1 {
list()
os.Exit(0)
}

if flag.NArg() == 1 {
switch flag.Arg(0) {
case "list", "l":
list()
case "appendPathctlDir", "apd":
appendPath(exePath())
case "prependPathctlDir", "ppd":
prepend(exePath())
default:
log.Fatalf("unrecognized command: %s", flag.Arg(0))
}
} else {
switch flag.Arg(0) {
case "prepend", "p":
prepend(flag.Arg(1))
case "drop", "d":
drop(flag.Arg(1))
case "append", "a":
appendPath(flag.Arg(1))
}
}

fmt.Println(paths.String())
}
Comment on lines +38 to +76
Copy link
Contributor

Choose a reason for hiding this comment

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

The main function could be refactored to improve readability and maintainability. The current implementation has nested if and switch statements, which can be simplified by breaking down the command handling into separate functions or by using a map of functions indexed by command names.

// Example of a possible refactoring using a map of functions:
var commandHandlers = map[string]func(){
	"list": func() { list() },
	"appendPathctlDir": func() { appendPath(exePath()) },
	"prependPathctlDir": func() { prepend(exePath()) },
	"prepend": func() { prepend(flag.Arg(1)) },
	"drop": func() { drop(flag.Arg(1)) },
	"append": func() { appendPath(flag.Arg(1)) },
}

func main() {
	// ... existing code ...

	if handler, ok := commandHandlers[flag.Arg(0)]; ok {
		handler()
	} else {
		log.Fatalf("unrecognized command: %s", flag.Arg(0))
	}

	fmt.Println(paths.String())
}

This approach would make it easier to add or modify commands in the future and would make the main function more concise.


func list() {
for _, p := range paths.StringSlice() {
fmt.Println(p)
}
}

func prepend(p string) {
//oldPath := pathEnvvar
if ok := paths.Prepend(p); !ok {
Copy link
Contributor

Choose a reason for hiding this comment

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

The error messages in the prepend, drop, and appendPath functions are all the same and could be more descriptive. It would be beneficial to include the command that failed and the path that was attempted to be modified for better user feedback.

- log.Println("the path already exists")
+ log.Printf("failed to %s: the path '%s' already exists", <command>, p)

Replace <command> with the appropriate operation (e.g., "prepend", "drop", "append") to provide more context in the error message.

Also applies to: 92-92, 98-98


Committable suggestion

IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
if ok := paths.Prepend(p); !ok {
if ok := paths.Prepend(p); !ok {
log.Printf("failed to prepend: the path '%s' already exists", p)

log.Println("the path already exists")
}
}

func drop(p string) {
if ok := paths.Drop(p); !ok {
log.Println("the path already exists")
}
}

func appendPath(p string) {
if ok := paths.Append(p); !ok {
log.Println("the path already exists")
}
}

func handleHelpAndVersionModes() {
if helpMode {
usage()
os.Exit(0)
}

if versionMode {
version.PrintWithCopyright()
os.Exit(0)
}
}

func usage() {
s := fmt.Sprintf(`Usage: %s COMMAND [PATH]
Make the management of the PATH environment variable
simple, fast, and predictable.

Commands:

append, a append a path to the end
drop, d drop a path
list, l list the paths
prepend, p prepend a path to the list

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

flag.PrintDefaults()

_, _ = fmt.Fprintln(os.Stderr, `
If COMMAND is not provided, it prints the contents of the PATH
environment variable; the default output format is one path per
line.`)
}

func exePath() string {
exePath, err := os.Executable()
if err != nil {
log.Fatal(err)
}

return filepath.Dir(exePath)
}
10 changes: 9 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
module github.com/alessio/unixtools

go 1.17
go 1.21

require github.com/stretchr/testify v1.8.4

require (
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
10 changes: 10 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
10 changes: 10 additions & 0 deletions internal/path/dirlist.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package path

// List handles a list of directories in a predictable way.
type List interface {
String() interface{} //
Copy link
Contributor

Choose a reason for hiding this comment

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

The String() method in the List interface should return a string type instead of interface{} for clarity and consistency with typical string conversion methods.

- String() interface{} //
+ String() string

Committable suggestion

IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
String() interface{} //
String() string

StringSlice() []string
Prepend(path string) bool
Append(path string) bool
Drop(path string) bool
}
79 changes: 79 additions & 0 deletions internal/path/list.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
package path

import (
"os"
"path/filepath"
"slices"
"strings"
)

var ListSeparator = string(os.PathListSeparator)

type dirList struct {
lst []string
}

func newDirList(lst []string) *dirList {
return &dirList{lst: lst}
}

func NewPathList(v string) List {
return newDirList(makePathList(os.Getenv(v)))
}

func (p *dirList) String() interface{} {
return strings.Join(p.lst, ListSeparator)
Comment on lines +24 to +25
Copy link
Contributor

Choose a reason for hiding this comment

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

The String method of dirList should return a string type instead of interface{} to match its intended usage.

- func (p *dirList) String() interface{} {
+ func (p *dirList) String() string {

Committable suggestion

IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
func (p *dirList) String() interface{} {
return strings.Join(p.lst, ListSeparator)
func (p *dirList) String() string {
return strings.Join(p.lst, ListSeparator)

}

func (p *dirList) StringSlice() []string {
return p.lst
}

func (p *dirList) Prepend(path string) bool {
cleanPath := normalizePath(path)
if idx := slices.Index(p.lst, cleanPath); idx == -1 {
p.lst = append([]string{cleanPath}, p.lst...)
return true
}

return false
}

func (p *dirList) Append(path string) bool {
cleanPath := normalizePath(path)
if idx := slices.Index(p.lst, cleanPath); idx == -1 {
p.lst = append(p.lst, cleanPath)
return true
}

return false
}

func (p *dirList) Drop(path string) bool {
cleanPath := normalizePath(path)
if idx := slices.Index(p.lst, cleanPath); idx != -1 {
p.lst = slices.Delete(p.lst, idx, idx+1)
return true
}

return false
}

func makePathList(pathStr string) []string {
if pathStr == "" {
return nil
}

rawList := strings.Split(pathStr, ListSeparator)
cleanList := make([]string, len(rawList))

for i, s := range rawList {
cleanList[i] = normalizePath(s)
}

return cleanList
}

func normalizePath(s string) string {
return filepath.Clean(s)
}
54 changes: 54 additions & 0 deletions internal/path/list_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package path_test

import (
"fmt"
"github.com/alessio/unixtools/internal/path"
"github.com/stretchr/testify/require"
"testing"
)

func TestPathList_Prepend(t *testing.T) {
envVarName := fmt.Sprintf("TEST_%s", t.Name())
t.Setenv(envVarName, "/var/:/root/config:/Programs///")
lst := path.NewPathList(envVarName)

require.Equal(t, lst.String(), "/var:/root/config:/Programs")

require.True(t, lst.Prepend("/usr/local/go/bin"))
require.False(t, lst.Prepend("/usr/local/go/bin"))
require.False(t, lst.Prepend("/usr///local///go/bin/"))

require.Equal(t, "/usr/local/go/bin:/var:/root/config:/Programs", lst.String())
require.Equal(t, []string{"/usr/local/go/bin", "/var", "/root/config", "/Programs"}, lst.StringSlice())
}

func TestPathList_Append(t *testing.T) {
envVarName := fmt.Sprintf("TEST_%s", t.Name())
t.Setenv(envVarName, "/var/:/root/config:/Programs///")
lst := path.NewPathList(envVarName)

require.Equal(t, "/var:/root/config:/Programs", lst.String())

require.True(t, lst.Append("/usr/local/go/bin"))
require.False(t, lst.Append("/usr/local/go/bin"))
require.False(t, lst.Append("/usr///local///go/bin/"))

require.Equal(t, "/var:/root/config:/Programs:/usr/local/go/bin", lst.String())
require.Equal(t, []string{"/var", "/root/config", "/Programs", "/usr/local/go/bin"}, lst.StringSlice())
}

func TestPathList_Drop(t *testing.T) {
envVarName := fmt.Sprintf("TEST_%s", t.Name())
t.Setenv(envVarName,
"/usr/local/bin:/home/user/.local/bin/:/usr/local/sbin:/var:/root")
lst := path.NewPathList(envVarName)

require.Equal(t, "/usr/local/bin:/home/user/.local/bin:/usr/local/sbin:/var:/root", lst.String())
require.False(t, lst.Drop("/etc")) // non existing
require.True(t, lst.Drop("/home/user/.local/bin"))
require.False(t, lst.Drop("/home/user/.local/bin"))
require.True(t, lst.Drop("/root/./"))

require.Equal(t, "/usr/local/bin:/usr/local/sbin:/var", lst.String())
require.Equal(t, []string{"/usr/local/bin", "/usr/local/sbin", "/var"}, lst.StringSlice())
}
5 changes: 0 additions & 5 deletions internal/path/path.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package path

import (
"path"
"strings"
)

Expand Down Expand Up @@ -46,7 +45,3 @@ func RemoveDir(path string, s string) string {

return strings.Join(newPath, ":")
}

func normalizePath(s string) string {
return path.Clean(strings.TrimRight(s, "/"))
}
Loading