From 36ad4eb145d282b92d3e5d28fc536dbed60e15ee Mon Sep 17 00:00:00 2001 From: Dan McArdle Date: Thu, 11 Apr 2024 11:06:33 -0400 Subject: [PATCH] cmd/gitannex: Simplify messageParser's finalParameter() func Issue #7625 --- cmd/gitannex/gitannex.go | 36 +++++++++++++++++------------------ cmd/gitannex/gitannex_test.go | 21 +++++++------------- 2 files changed, 25 insertions(+), 32 deletions(-) diff --git a/cmd/gitannex/gitannex.go b/cmd/gitannex/gitannex.go index 333b6e97a..a71b04871 100644 --- a/cmd/gitannex/gitannex.go +++ b/cmd/gitannex/gitannex.go @@ -96,15 +96,15 @@ func (m *messageParser) nextSpaceDelimitedParameter() (string, error) { } // finalParameter consumes the final parameter, which may contain spaces. -func (m *messageParser) finalParameter() (string, error) { +func (m *messageParser) finalParameter() string { m.line = strings.TrimRight(m.line, "\r\n") if len(m.line) == 0 { - return "", errors.New("nothing remains to parse") + return "" } param := m.line m.line = "" - return param, nil + return param } // configDefinition describes a configuration value required by this command. We @@ -203,11 +203,7 @@ func (s *server) run() error { case "REMOVE": err = s.handleRemove(message) case "ERROR": - errorMessage, parseErr := message.finalParameter() - if parseErr != nil { - err = fmt.Errorf("error while parsing ERROR message from git-annex: %w", parseErr) - break - } + errorMessage := message.finalParameter() err = fmt.Errorf("received error message from git-annex: %s", errorMessage) // @@ -310,8 +306,8 @@ func (s *server) queryConfigs() error { return fmt.Errorf("failed to parse config value: %s %s", valueKeyword, message.line) } - value, err := message.finalParameter() - if err != nil || value == "" { + value := message.finalParameter() + if value == "" { return fmt.Errorf("config value of %q must not be empty", config.name) } @@ -351,10 +347,10 @@ func (s *server) handleTransfer(message *messageParser) error { s.sendMsg("TRANSFER-FAILURE failed to parse key") return fmt.Errorf("malformed arguments for TRANSFER: %w", err) } - argFile, err := message.finalParameter() - if err != nil { - s.sendMsg("TRANSFER-FAILURE failed to parse file") - return fmt.Errorf("malformed arguments for TRANSFER: %w", err) + argFile := message.finalParameter() + if argFile == "" { + s.sendMsg("TRANSFER-FAILURE failed to parse file path") + return errors.New("failed to parse file path") } if err := s.queryConfigs(); err != nil { @@ -409,9 +405,9 @@ func (s *server) handleTransfer(message *messageParser) error { } func (s *server) handleCheckPresent(message *messageParser) error { - argKey, err := message.finalParameter() - if err != nil { - return err + argKey := message.finalParameter() + if argKey == "" { + return errors.New("failed to parse response for CHECKPRESENT") } if err := s.queryConfigs(); err != nil { @@ -440,7 +436,11 @@ func (s *server) handleCheckPresent(message *messageParser) error { } func (s *server) handleRemove(message *messageParser) error { - argKey, err := message.finalParameter() + argKey := message.finalParameter() + if argKey == "" { + return errors.New("failed to parse key for REMOVE") + } + if err != nil { return err } diff --git a/cmd/gitannex/gitannex_test.go b/cmd/gitannex/gitannex_test.go index f0b9b6c19..b3846221d 100644 --- a/cmd/gitannex/gitannex_test.go +++ b/cmd/gitannex/gitannex_test.go @@ -64,12 +64,10 @@ var messageParserTestCases = []messageParserTestCase{ assert.Error(t, err) assert.Equal(t, param, "") - param, err = m.finalParameter() - assert.Error(t, err) + param = m.finalParameter() assert.Equal(t, param, "") - param, err = m.finalParameter() - assert.Error(t, err) + param = m.finalParameter() assert.Equal(t, param, "") param, err = m.nextSpaceDelimitedParameter() @@ -95,8 +93,7 @@ var messageParserTestCases = []messageParserTestCase{ assert.Error(t, err) assert.Equal(t, param, "") - param, err = m.finalParameter() - assert.Error(t, err) + param = m.finalParameter() assert.Equal(t, param, "") }, }, @@ -118,8 +115,7 @@ var messageParserTestCases = []messageParserTestCase{ assert.Error(t, err) assert.Equal(t, param, "") - param, err = m.finalParameter() - assert.Error(t, err) + param = m.finalParameter() assert.Equal(t, param, "") }, }, @@ -136,8 +132,7 @@ var messageParserTestCases = []messageParserTestCase{ assert.NoError(t, err) assert.Equal(t, param, "secondparam") - param, err = m.finalParameter() - assert.NoError(t, err) + param = m.finalParameter() assert.Equal(t, param, "final param with spaces") }, }, @@ -151,12 +146,10 @@ var messageParserTestCases = []messageParserTestCase{ t.Run(testName, func(t *testing.T) { m := messageParser{"one long final parameter" + lineEnding} - param, err := m.finalParameter() - assert.NoError(t, err) + param := m.finalParameter() assert.Equal(t, param, "one long final parameter") - param, err = m.finalParameter() - assert.Error(t, err) + param = m.finalParameter() assert.Equal(t, param, "") })