diff --git a/bin/cli-app-linux-amd64 b/bin/cli-app-linux-amd64 old mode 100644 new mode 100755 index 86371a8..dff3cf7 Binary files a/bin/cli-app-linux-amd64 and b/bin/cli-app-linux-amd64 differ diff --git a/docs/review.md b/docs/review.md new file mode 100644 index 0000000..94372ef --- /dev/null +++ b/docs/review.md @@ -0,0 +1,38 @@ +# Ревью проекта + +## Архитектура +### Плюсы +- универсальная/гибкая сигнатура функции команды +- хорошее распределение ответственности между классами + +### Минусы +- почти нет; изначальная сигнатура команды: + ``` + func name_command(parseline.Command, *bytes.Buffer) (*bytes.Buffer, error) {} + ``` + не позволяет реализовывать команды, меняющие состояние Executor + +## Реализация +### Плюсы +- легкость добавления новых команд: 1 строка в newCommands() + 1 новая функция со стандартной сигнатурой +- хорошие, подробные тесты для парсера + +### Минусы +- плохое разбиение по файлам: все команды в одном файле, все тесты для команд в одном файле, все служебные функции для реализации команд в одном файле +- служебные функции для реализации конкретных команд никак не выделены: нет документации, по названию непонятно, где она нужна, лежат в одном файле с реализацией других команд +- плохая документация тестов: ее почти нет +- плохая документация реализации команд: ее нет вообще +- (pet peeve) комментарии в коде на двух языках + +## Репозиторий +### Плюсы +- подробная документация в папке docs: расписаны существующие команды, архитектура проекта, гайд по добавлению новых команд - всё здорово + +### Минусы +- Изначально команды, которые работал с файлами, могли работать только в текущей директории и не учитывали переходы между ними. +- нет плашки с тестами в README +- (pet peeve) нет ссылок на документы из docs/папку docs в головном README + +## Рекомендации +- можно разбить commands.go на пакеты/файлы +- добавить комментарии к служебным функциям, выделить их в отдельный пакет/файл diff --git a/internal/environment/environment.go b/internal/environment/environment.go index 45c2b9e..1253584 100644 --- a/internal/environment/environment.go +++ b/internal/environment/environment.go @@ -7,7 +7,6 @@ import ( type Env map[string]string - // Constructor of environment func New() Env { env := Env{} @@ -18,7 +17,7 @@ func New() Env { cmd := os.Getenv(v) env[v] = string(cmd) } - + return env } @@ -33,6 +32,7 @@ func (env Env) Reset() { env[v] = string(cmd) } } + // Set a new variable func (env Env) Set(variable, value string) { env[variable] = value @@ -44,4 +44,4 @@ func (env Env) Get(variable string) (string, error) { return v, nil } return "", fmt.Errorf("unknown command: %s", variable) -} \ No newline at end of file +} diff --git a/internal/executor/commands.go b/internal/executor/commands.go index abc8bb9..4b2cd34 100644 --- a/internal/executor/commands.go +++ b/internal/executor/commands.go @@ -3,22 +3,22 @@ package executor import ( "CLI/internal/parseline" "bytes" + "errors" "fmt" "os" - "strings" - "errors" - "strconv" + "path/filepath" "regexp" + "strconv" + "strings" "github.com/spf13/pflag" ) type commands map[string]func(parseline.Command, *bytes.Buffer) (*bytes.Buffer, error) - func newCommands() commands { cmds := make(commands) - + // Here you can add new command in CLI // cmd["name_command"] = name_command // below you need to implement a command with the following signature: @@ -29,7 +29,8 @@ func newCommands() commands { cmds["pwd"] = pwd cmds["wc"] = wc cmds["grep"] = grep - + cmds["cd"] = cd + cmds["ls"] = ls return cmds } @@ -39,7 +40,7 @@ func newCommands() commands { func cat(cmd parseline.Command, b *bytes.Buffer) (*bytes.Buffer, error) { output := bytes.NewBuffer(nil) - if len(cmd.Args) == 0 { + if len(cmd.Args) == 1 { if b != nil { _, err := output.Write(b.Bytes()) return output, err @@ -47,8 +48,12 @@ func cat(cmd parseline.Command, b *bytes.Buffer) (*bytes.Buffer, error) { return nil, errors.New("no input provided") } - for _, filename := range cmd.Args { - data, err := os.ReadFile(filename) + for _, filename := range cmd.Args[1:] { + abs_path, _ := filepath.Abs(filepath.Join(cmd.Args[0], filename)) // getting current directory path + filename + data, err := os.ReadFile(abs_path) + if err != nil { + data, err = os.ReadFile(filename) + } if err != nil { return nil, fmt.Errorf("cat: %w", err) } @@ -65,10 +70,10 @@ func echo(cmd parseline.Command, b *bytes.Buffer) (*bytes.Buffer, error) { content := strings.Join(cmd.Args, " ") if content[0] == '"' { - content = content[1:len(content) - 1] + content = content[1 : len(content)-1] } if content[0] == '\'' { - content = content[1:len(content) - 1] + content = content[1 : len(content)-1] content = strings.ReplaceAll(content, "\\n", "\n") } b.Reset() @@ -85,24 +90,26 @@ func exit(cmd parseline.Command, b *bytes.Buffer) (*bytes.Buffer, error) { } } os.Exit(code) - return nil, nil + return nil, nil } func pwd(cmd parseline.Command, _ *bytes.Buffer) (*bytes.Buffer, error) { - dir, err := os.Getwd() - if err != nil { + var dir string + dir, err := filepath.Abs(cmd.Args[0]) + if err != nil { return nil, fmt.Errorf("pwd: %w", err) } - output := bytes.NewBufferString(dir) + output := bytes.NewBufferString(dir) return output, nil } func wc(cmd parseline.Command, b *bytes.Buffer) (*bytes.Buffer, error) { var input string - if len(cmd.Args) == 0 { + if len(cmd.Args) == 1 { input = b.String() - } else if len(cmd.Args) > 0 { - data, err := os.ReadFile(cmd.Args[0]) + } else if len(cmd.Args) > 1 { + abs_path, _ := filepath.Abs(filepath.Join(cmd.Args[0], cmd.Args[1])) // getting current directory path + filename + data, err := os.ReadFile(abs_path) if err != nil { return nil, fmt.Errorf("wc: %w", err) } @@ -141,7 +148,7 @@ func grep(cmd parseline.Command, input *bytes.Buffer) (*bytes.Buffer, error) { func parseArgs(args []string) (*grepOptions, error) { opts := &grepOptions{} fs := pflag.NewFlagSet("grep", pflag.ContinueOnError) - + fs.IntVarP(&opts.afterContext, "after-context", "A", 0, "Lines after match") fs.BoolVarP(&opts.ignoreCase, "ignore-case", "i", false, "Ignore case") fs.BoolVarP(&opts.wordRegexp, "word-regexp", "w", false, "Whole word match") @@ -165,11 +172,11 @@ func parseArgs(args []string) (*grepOptions, error) { func compileRegex(opts *grepOptions) (*regexp.Regexp, error) { pattern := opts.pattern - + if opts.wordRegexp { pattern = fmt.Sprintf(`\b%s\b`, pattern) } - + if opts.ignoreCase { pattern = "(?i)" + pattern } @@ -212,4 +219,43 @@ func processData(data []byte, re *regexp.Regexp, context int) *bytes.Buffer { } return &output -} \ No newline at end of file +} + +func cd(cmd parseline.Command, b *bytes.Buffer) (*bytes.Buffer, error) { + var path string + if len(cmd.Args) == 1 { + homeDir, err := os.UserHomeDir() + if err != nil { + return nil, fmt.Errorf("cd: cannot get home directory: %w", err) + } + path = homeDir + } else { + path, _ = filepath.Abs(filepath.Join(cmd.Args[0], cmd.Args[1])) + } + + info, err := os.Stat(path) + if err != nil || !info.IsDir() { + return nil, fmt.Errorf("cd: %s: not a directory", path) + } + + b.WriteString(path) + return b, nil +} + +// Takes first provided argument as path to a directory and lists all files in it. +func ls(cmd parseline.Command, buffer *bytes.Buffer) (*bytes.Buffer, error) { + path := cmd.Args[0] + if len(cmd.Args) == 2 { + path, _ = filepath.Abs(filepath.Join(cmd.Args[0], cmd.Args[1])) // getting correct path + } + entries, err := os.ReadDir(path) + if err != nil { + return nil, fmt.Errorf("ls: %w", err) + } + + for _, entry := range entries { + buffer.WriteString(entry.Name() + "\n") + } + + return buffer, nil +} diff --git a/internal/executor/commands_test.go b/internal/executor/commands_test.go index ff35d82..1195982 100644 --- a/internal/executor/commands_test.go +++ b/internal/executor/commands_test.go @@ -1,14 +1,21 @@ package executor import ( + "CLI/internal/environment" "CLI/internal/parseline" "bytes" "os" + "path/filepath" + "slices" + "strings" "testing" ) func TestCat(t *testing.T) { - tmpfile, err := os.CreateTemp("", "testfile") + env := environment.New() + exec := New(env) + exec.cwd = t.TempDir() + tmpfile, err := os.CreateTemp(exec.cwd, "testfile") if err != nil { t.Fatal(err) } @@ -24,14 +31,14 @@ func TestCat(t *testing.T) { }{ { name: "Read file", - cmd: parseline.Command{Name: "cat", Args: []string{tmpfile.Name()}}, + cmd: parseline.Command{Name: "cat", Args: []string{"", tmpfile.Name()}}, input: nil, want: "test data\n", wantErr: false, }, { name: "No file", - cmd: parseline.Command{Name: "cat", Args: []string{"nonexistent.txt"}}, + cmd: parseline.Command{Name: "cat", Args: []string{"nonexistent.txt", exec.cwd}}, input: nil, want: "", wantErr: true, @@ -98,8 +105,12 @@ func TestEcho(t *testing.T) { } func TestPwd(t *testing.T) { - want, _ := os.Getwd() + env := environment.New() + exec := New(env) + exec.cwd = t.TempDir() + want := exec.cwd cmd := parseline.Command{Name: "pwd", Args: nil} + cmd.Args = append(cmd.Args, exec.cwd) got, err := pwd(cmd, nil) if err != nil { t.Errorf("pwd() error = %v", err) @@ -110,6 +121,10 @@ func TestPwd(t *testing.T) { } func TestWc(t *testing.T) { + env := environment.New() + exec := New(env) + exec.cwd = t.TempDir() + tests := []struct { name string cmd parseline.Command @@ -135,6 +150,7 @@ func TestWc(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + tt.cmd.Args = append(tt.cmd.Args, exec.cwd) got, err := wc(tt.cmd, tt.input) if (err != nil) != tt.wantErr { t.Errorf("wc() error = %v, wantErr %v", err, tt.wantErr) @@ -284,3 +300,135 @@ func TestGrepInvalidRegex(t *testing.T) { } } +func TestLsEmpty(t *testing.T) { + tempDir := t.TempDir() + + cmd := parseline.Command{ + Name: "ls", + Args: []string{tempDir}, + } + var testInput []byte + input := bytes.NewBuffer(testInput) + expected := `` + + output, err := ls(cmd, input) + if err != nil { + t.Fatalf("ls failed: %v", err) + } + if output.String() != expected { + t.Errorf("Expected:\n%s\nGot:\n%s", expected, output.String()) + } +} + +func TestLsNonEmpty(t *testing.T) { + tempDir := t.TempDir() + var filePaths []string + for i := 0; i < 5; i++ { + file, err := os.CreateTemp(tempDir, "testfile-*.tmp") + if err != nil { + t.Fatalf("Failed to create temporary file: %v", err) + } + defer file.Close() + filePaths = append(filePaths, filepath.Base(file.Name())) + } + + cmd := parseline.Command{ + Name: "ls", + Args: []string{tempDir}, + } + var testInput []byte + input := bytes.NewBuffer(testInput) + slices.Sort(filePaths) + expected := strings.Join(filePaths, "\n") + "\n" + + output, err := ls(cmd, input) + if err != nil { + t.Fatalf("ls failed: %v", err) + } + if output.String() != expected { + t.Errorf("Expected:\n%s\nGot:\n%s", expected, output.String()) + } +} + +func TestLsCurrentDir(t *testing.T) { + cmd := parseline.Command{ + Name: "ls", + } + var testInput []byte + input := bytes.NewBuffer(testInput) + + arg, _ := os.Getwd() + cmd.Args = append([]string{arg}, cmd.Args...) + output, err := ls(cmd, input) + if err != nil { + t.Fatalf("ls failed: %v", err) + } + output_tokens := strings.Split(output.String(), "\n") + t.Log(output_tokens) + if !slices.Contains(output_tokens, "commands_test.go") { + t.Errorf("ls without argument does not work") + } +} +func TestCdCasualUsage(t *testing.T) { + tmpDir := t.TempDir() + subDir := filepath.Join(tmpDir, "subdir") + if err := os.Mkdir(subDir, 0755); err != nil { + t.Fatalf("failed to create subdir: %v", err) + } + + env := environment.New() + exec := New(env) + exec.cwd = tmpDir + + if exec.cwd != tmpDir { + t.Fatalf("expected cwd to be %s, got %s", tmpDir, exec.cwd) + } + + cmd := parseline.Command{Name: "cd", Args: []string{"subdir"}} + arg := tmpDir + cmd.Args = append([]string{arg}, cmd.Args...) + res, err := cd(cmd, &bytes.Buffer{}) + if err != nil { + t.Fatalf("cd command failed: %v", err) + } + + if res.String() != subDir { + t.Errorf("expected cwd to be %s after cd, got %s", subDir, res.String()) + } +} + +func TestCdToHome(t *testing.T) { + homeDir, err := os.UserHomeDir() + if err != nil { + t.Skip("can't test home dir; os.UserHomeDir() failed") + } + + env := environment.New() + exec := New(env) + exec.cwd = "/" + + cmd := parseline.Command{Name: "cd"} // no args + arg := homeDir + cmd.Args = append(cmd.Args, arg) + res, err := cd(cmd, &bytes.Buffer{}) + if err != nil { + t.Fatalf("cd to home failed: %v", err) + } + if res.String() != homeDir { + t.Errorf("expected home dir %q, got %q", homeDir, exec.cwd) + } +} + +func TestCdToInvalidPath(t *testing.T) { + env := environment.New() + exec := New(env) + exec.cwd = t.TempDir() + + cmd := parseline.Command{Name: "cd", Args: []string{"not-a-dir"}} + arg := t.TempDir() + cmd.Args = append(cmd.Args, arg) + _, err := cd(cmd, &bytes.Buffer{}) + if err == nil { + t.Fatal("expected error when cd into nonexistent dir, got none") + } +} diff --git a/internal/executor/executor.go b/internal/executor/executor.go index 8202dfc..2be2ea9 100644 --- a/internal/executor/executor.go +++ b/internal/executor/executor.go @@ -5,6 +5,7 @@ import ( "CLI/internal/parseline" "bytes" "fmt" + "os" "os/exec" "strings" ) @@ -12,16 +13,22 @@ import ( // Executor stores a self-implemented functions and a reference to an object storing environment variables type Executor struct { cmds commands - env environment.Env + env environment.Env + cwd string } // Constructor: creates a new Executor and initializes commands // Parameters: // - env: environment.Env func New(env environment.Env) *Executor { + startDir, err := os.Getwd() + if err != nil { + startDir = "/" + } return &Executor{ - env: env, + env: env, cmds: newCommands(), + cwd: startDir, } } @@ -41,8 +48,7 @@ func (executor *Executor) Execute(commands []parseline.Command) (*bytes.Buffer, } } return buffer, nil -} - +} type grepOptions struct { ignoreCase bool @@ -52,33 +58,48 @@ type grepOptions struct { files []string } - func (executor *Executor) execute(command parseline.Command, b *bytes.Buffer) (*bytes.Buffer, error) { + var result *bytes.Buffer + var res_error error if cmd, ok := executor.cmds[command.Name]; ok { - return cmd(command, b) - } else if strings.ContainsRune(command.Name, '=' ){ + // Adding current dir path as first argument for commands that relies in current pwd + if command.Name == "ls" || command.Name == "cd" || + command.Name == "pwd" || command.Name == "cat" || + command.Name == "wc" { + command.Args = append([]string{executor.cwd}, command.Args...) + } + result, res_error = cmd(command, b) + if command.Name == "cd" && result != nil { + executor.env.Set("OLD_PWD", executor.cwd) + executor.cwd = result.String() + executor.env.Set("PWD", executor.cwd) + return &bytes.Buffer{}, res_error + } + return result, res_error + } else if strings.ContainsRune(command.Name, '=') { split := strings.Split(command.Name, "=") if len(split) != 2 { return nil, fmt.Errorf("command %s: '=' is incorrect symbol for variable or value", command.Name) - } + } if len(split[0]) == 0 || len(split[1]) == 0 { return nil, fmt.Errorf("command %s: incorrect lenght of variable or value", command.Name) - } + } executor.env.Set(split[0], split[1]) return bytes.NewBufferString(""), nil } else { var res *exec.Cmd - content := b.String() + content := b.String() if len(content) > 0 { res = exec.Command(command.Name, append(command.Args, content)...) } else { res = exec.Command(command.Name, command.Args...) } + res.Dir = executor.cwd output, err := res.Output() if err != nil { return nil, fmt.Errorf("command %s: %s", command.Name, err.Error()) } return bytes.NewBufferString(string(output)), nil } -} \ No newline at end of file +} diff --git a/internal/executor/executor_test.go b/internal/executor/executor_test.go index a03b96f..4bc9014 100644 --- a/internal/executor/executor_test.go +++ b/internal/executor/executor_test.go @@ -3,71 +3,72 @@ package executor import ( "CLI/internal/environment" "CLI/internal/parseline" - "testing" "os" + "testing" ) func TestPipeline_EchoToWc(t *testing.T) { env := environment.New() executor := New(env) - commands := []parseline.Command{ - {Name: "echo", Args: []string{"hello\nworld\n"}}, - {Name: "wc", Args: []string{}}, - } + commands := []parseline.Command{ + {Name: "echo", Args: []string{"hello\nworld\n"}}, + {Name: "wc", Args: []string{}}, + } - result, err := executor.Execute(commands) - if err != nil { - t.Fatalf("ExecutePipeline failed: %v", err) - } + result, err := executor.Execute(commands) + if err != nil { + t.Fatalf("ExecutePipeline failed: %v", err) + } - expected := "2 2 12" - if result.String() != expected { - t.Errorf("Expected %q, got %q", expected, result.String()) - } + expected := "2 2 12" + if result.String() != expected { + t.Errorf("Expected %q, got %q", expected, result.String()) + } } func TestPipeline_CatToWc(t *testing.T) { - env := environment.New() + env := environment.New() executor := New(env) - tmpfile, err := os.CreateTemp("", "testfile") - if err != nil { - t.Fatal(err) - } - defer os.Remove(tmpfile.Name()) - tmpfile.WriteString("hello\nworld\n") + executor.cwd = t.TempDir() + tmpfile, err := os.CreateTemp(executor.cwd, "testfile") + if err != nil { + t.Fatal(err) + } + defer os.Remove(tmpfile.Name()) + tmpfile.WriteString("hello\nworld\n") - commands := []parseline.Command{ - {Name: "cat", Args: []string{tmpfile.Name()}}, - {Name: "wc", Args: []string{}}, - } + commands := []parseline.Command{ + {Name: "cat", Args: []string{tmpfile.Name()}}, + {Name: "wc", Args: []string{}}, + } - result, err := executor.Execute(commands) - if err != nil { - t.Fatalf("ExecutePipeline failed: %v", err) - } + result, err := executor.Execute(commands) + if err != nil { + t.Fatalf("ExecutePipeline failed: %v", err) + } - expected := "2 2 12" - if result.String() != expected { - t.Errorf("Expected %q, got %q", expected, result.String()) - } + expected := "2 2 12" + if result.String() != expected { + t.Errorf("Expected %q, got %q", expected, result.String()) + } } func TestPipeline_EchoCatWc(t *testing.T) { env := environment.New() executor := New(env) - commands := []parseline.Command{ - {Name: "echo", Args: []string{"hello\nworld\n"}}, - {Name: "cat", Args: []string{}}, - {Name: "wc", Args: []string{}}, - } + commands := []parseline.Command{ + {Name: "echo", Args: []string{"hello\nworld\n"}}, + {Name: "cat", Args: []string{}}, + {Name: "wc", Args: []string{}}, + } - result, err := executor.Execute(commands) - if err != nil { - t.Fatalf("ExecutePipeline failed: %v", err) - } + result, err := executor.Execute(commands) + if err != nil { + t.Fatalf("ExecutePipeline failed: %v", err) + } - expected := "2 2 12" - if result.String() != expected { - t.Errorf("Expected %q, got %q", expected, result.String()) - } -} \ No newline at end of file + expected := "2 2 12" + if result.String() != expected { + t.Errorf("Expected %q, got %q", expected, result.String()) + } +} diff --git a/internal/parseline/parser.go b/internal/parseline/parser.go index 15d28f6..0a8b41b 100644 --- a/internal/parseline/parser.go +++ b/internal/parseline/parser.go @@ -2,9 +2,9 @@ package parseline import ( "CLI/internal/environment" + "errors" "fmt" "strings" - "errors" "unicode" ) @@ -13,26 +13,27 @@ type Parser struct { env environment.Env } -// Constructor of parser +// Constructor of parser // Parameters: env environment.Env func New(env environment.Env) *Parser { return &Parser{ env: env, } } + // Command store name of command and it's flags and args type Command struct { - Name string - Args []string + Name string + Args []string } // ParsePipeline: parses the received string into pipeline -// Parameters: -// - input: string -// Returns: +// Parameters: +// - input: string +// Returns: // - []Command: pipeline // - error: -func (parser * Parser)ParsePipeline(input string) ([]Command, error) { +func (parser *Parser) ParsePipeline(input string) ([]Command, error) { var commands []Command var currentCmd strings.Builder var inSingleQuote, inDoubleQuote, escaped bool @@ -123,17 +124,17 @@ func (parser *Parser) substitution(s string) (string, error) { i := 0 n := len(s) inQuotes := false - quoteChar := byte(0) + quoteChar := byte(0) for i < n { switch { case s[i] == '"': if !inQuotes { - + inQuotes = true quoteChar = s[i] } else if s[i] == quoteChar { - + inQuotes = false quoteChar = 0 } @@ -193,4 +194,4 @@ func (parser *Parser) substitution(s string) (string, error) { func isAlphaNum(c byte) bool { return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') -} \ No newline at end of file +} diff --git a/.gitignore b/scripts/.gitignore similarity index 100% rename from .gitignore rename to scripts/.gitignore