Skip to content

stdbin: adding mkdir #240

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

Merged
merged 5 commits into from
Oct 7, 2017
Merged
Show file tree
Hide file tree
Changes from 2 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
6 changes: 4 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ buildargs = -ldflags "-X main.VersionString=$(version)" -v
all: build test install

build:
go build $(buildargs) -o ./cmd/nash/nash ./cmd/nash
go build $(buildargs) -o ./cmd/nashfmt/nashfmt ./cmd/nashfmt
cd cmd/nash && go build $(buildargs)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this change was needed because on windows the generated binaries must have .exe extensions.

cd cmd/nashfmt && go build $(buildargs)
cd stdbin/mkdir && go build $(buildargs)

NASHPATH=$(HOME)/nash
NASHROOT=$(HOME)/nashroot
Expand All @@ -23,6 +24,7 @@ install: build
cp -p ./cmd/nashfmt/nashfmt $(NASHROOT)/bin
rm -rf $(NASHROOT)/stdlib
cp -pr ./stdlib $(NASHROOT)/stdlib
cp -pr ./stdbin/mkdir/mkdir $(NASHROOT)/bin/mkdir
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this install still only works on unix, I wll address this in the windows branch soon.

Copy link
Member

Choose a reason for hiding this comment

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

thanks, I dont even have an env to test this kind of stuff =P (nor the patience)


deps:
go get -v -t golang.org/x/exp/ebnf
Expand Down
28 changes: 28 additions & 0 deletions stdbin/mkdir/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package main

import (
"os"
"fmt"
)

func mkdirs(dirnames []string) error {
for _, d := range dirnames {
if err := os.MkdirAll(d, 0644); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

cant you pass os.ModeDir here instead of 0644 ?

Copy link
Member

Choose a reason for hiding this comment

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

always creating all dirs seems nice, I always do it =).

another thing very common is to ignore if the dir already exists, in the end I always use "-p" for this. What you think about it being the default ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, makes sense. That's what I commonly do also.

Copy link
Collaborator Author

@i4ki i4ki Sep 27, 2017

Choose a reason for hiding this comment

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

cant you pass os.ModeDir here instead of 0644

0644 are the permission bits here, not the file mode. Sorry, did I misunderstand you?

Copy link
Member

Choose a reason for hiding this comment

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

From the name I thought the os.ModeDir would be an integer with the permission bits (indicating the default permissions for directories). My mistake =)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

👍

return err
}
}

return nil
}

func main() {
if len(os.Args) < 2 {
fmt.Fprintf(os.Stderr, "usage: %s <dir1> <dir2> ...\n", os.Args[0])
os.Exit(1)
}
err := mkdirs(os.Args[1:])
if err != nil {
fmt.Fprintf(os.Stderr, "error: %s\n", err.Error())
Copy link
Member

Choose a reason for hiding this comment

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

No need to call err.Error

os.Exit(1)
}
}
38 changes: 38 additions & 0 deletions stdbin/mkdir/mkdir_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package main

import (
"testing"
"path"
"path/filepath"
"io/ioutil"
"os"
"fmt"
)

func TestMkdir(t *testing.T) {
tmpDir, err := ioutil.TempDir("", "nash-mkdir")
if err != nil {
t.Fatal(err)
}

testDirs := []string{
"1", "2", "3", "4", "5",
"some", "thing", "_random_",
"_",
}

defer os.RemoveAll(tmpDir)
for _, p := range testDirs {
testdir := filepath.FromSlash(path.Join(tmpDir, p))
err = mkdirs([]string{testdir})
Copy link
Member

Choose a reason for hiding this comment

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

Isn't a good idea to test with arrays bigger than one too ? Also, what happens if the array is empty ? (can test it on the function or on the final cmd)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, there are missing test cases. I made this in a rush... =/
thanks

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for working on this =). We can add an TODO too ;-)

if err != nil {
t.Fatal(err)
}

if s, err := os.Stat(testdir); err != nil {
t.Fatal(err)
} else if s.Mode()&os.ModeDir != os.ModeDir {
t.Errorf("Invalid directory mode: %v", s.Mode())
Copy link
Member

Choose a reason for hiding this comment

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

Why Errorf instead of Fatalf since all other checks results on Fatal ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm idiot =)

Copy link
Member

Choose a reason for hiding this comment

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

auheauheauheuaheauheauhe yeah like no one else does this kind of thing =P

}
}
}