Skip to content

Some small refactoring on legacy package #1064

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.

Sign up for GitHub

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 9 commits into from
Nov 12, 2020
Merged

Some small refactoring on legacy package #1064

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
fe086e4
legacy: output --preprocess result on ExecStdout
cmaglie Nov 11, 2020
382d77c
legacy: removed redundant argument filters
cmaglie Nov 11, 2020
5880d01
legacy: moving path-relativization code out of PrepareCommand
cmaglie Nov 11, 2020
815deb7
legacy: removed i18n on unneded message
cmaglie Nov 11, 2020
552afac
legacy: replacing ParseCommandLine with the equivalent library call
cmaglie Nov 11, 2020
c32e0fd
legacy: removed parameter that happens to be always false
cmaglie Nov 11, 2020
6ff849f
legacy: removed constants.MSG_PATTERN_MISSING from i18n
cmaglie Nov 11, 2020
706d825
Let ExecRecipe return the command executed
matthijskooijman Sep 11, 2020
3d683f3
Fixed typo
cmaglie Nov 12, 2020
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
40 changes: 0 additions & 40 deletions legacy/builder/test/utils_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,39 +23,6 @@ import (
"github.com/stretchr/testify/require"
)

func TestCommandLineParser(t *testing.T) {
command := "\"/home/federico/materiale/works_Arduino/Arduino/build/hardware/tools/coan\" source -m -E -P -kb -c -g -Os -w -ffunction-sections -fdata-sections -MMD -mmcu=atmega32u4 -DF_CPU=16000000L -DARDUINO=010600 -DARDUINO_AVR_LEONARDO -DARDUINO_ARCH_AVR -DUSB_VID=0x2341 -DUSB_PID=0x8036 '-DUSB_MANUFACTURER=' '-DUSB_PRODUCT=\"Arduino Leonardo\"' \"/tmp/sketch321469072.cpp\""

parts, err := utils.ParseCommandLine(command)
NoError(t, err)

require.Equal(t, 23, len(parts))

require.Equal(t, "/home/federico/materiale/works_Arduino/Arduino/build/hardware/tools/coan", parts[0])
require.Equal(t, "source", parts[1])
require.Equal(t, "-m", parts[2])
require.Equal(t, "-E", parts[3])
require.Equal(t, "-P", parts[4])
require.Equal(t, "-kb", parts[5])
require.Equal(t, "-c", parts[6])
require.Equal(t, "-g", parts[7])
require.Equal(t, "-Os", parts[8])
require.Equal(t, "-w", parts[9])
require.Equal(t, "-ffunction-sections", parts[10])
require.Equal(t, "-fdata-sections", parts[11])
require.Equal(t, "-MMD", parts[12])
require.Equal(t, "-mmcu=atmega32u4", parts[13])
require.Equal(t, "-DF_CPU=16000000L", parts[14])
require.Equal(t, "-DARDUINO=010600", parts[15])
require.Equal(t, "-DARDUINO_AVR_LEONARDO", parts[16])
require.Equal(t, "-DARDUINO_ARCH_AVR", parts[17])
require.Equal(t, "-DUSB_VID=0x2341", parts[18])
require.Equal(t, "-DUSB_PID=0x8036", parts[19])
require.Equal(t, "-DUSB_MANUFACTURER=", parts[20])
require.Equal(t, "-DUSB_PRODUCT=\"Arduino Leonardo\"", parts[21])
require.Equal(t, "/tmp/sketch321469072.cpp", parts[22])
}

func TestPrintableCommand(t *testing.T) {
parts := []string{
"/path/to/dir with spaces/cmd",
Expand All @@ -75,13 +42,6 @@ func TestPrintableCommand(t *testing.T) {
require.Equal(t, correct, result)
}

func TestCommandLineParserError(t *testing.T) {
command := "\"command missing quote"

_, err := utils.ParseCommandLine(command)
require.Error(t, err)
}

func TestMapTrimSpace(t *testing.T) {
value := "hello, world , how are,you? "
parts := utils.Map(strings.Split(value, ","), utils.TrimSpace)
Expand Down
58 changes: 3 additions & 55 deletions legacy/builder/utils/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,56 +29,15 @@ import (
"unicode"
"unicode/utf8"

"github.com/arduino/arduino-cli/legacy/builder/constants"
"github.com/arduino/arduino-cli/legacy/builder/gohasissues"
"github.com/arduino/arduino-cli/legacy/builder/types"
paths "github.com/arduino/go-paths-helper"
"github.com/arduino/go-properties-orderedmap"
"github.com/pkg/errors"
"golang.org/x/text/transform"
"golang.org/x/text/unicode/norm"
)

func ParseCommandLine(input string) ([]string, error) {
var parts []string
escapingChar := constants.EMPTY_STRING
escapedArg := constants.EMPTY_STRING
for _, inputPart := range strings.Split(input, constants.SPACE) {
inputPart = strings.TrimSpace(inputPart)
if len(inputPart) == 0 {
continue
}

if escapingChar == constants.EMPTY_STRING {
if inputPart[0] != '"' && inputPart[0] != '\'' {
parts = append(parts, inputPart)
continue
}

escapingChar = string(inputPart[0])
inputPart = inputPart[1:]
escapedArg = constants.EMPTY_STRING
}

if inputPart[len(inputPart)-1] != '"' && inputPart[len(inputPart)-1] != '\'' {
escapedArg = escapedArg + inputPart + " "
continue
}

escapedArg = escapedArg + inputPart[:len(inputPart)-1]
escapedArg = strings.TrimSpace(escapedArg)
if len(escapedArg) > 0 {
parts = append(parts, escapedArg)
}
escapingChar = constants.EMPTY_STRING
}

if escapingChar != constants.EMPTY_STRING {
return nil, errors.Errorf("Invalid quoting: no closing [%s] char found.", escapingChar)
}

return parts, nil
}

type filterFiles func([]os.FileInfo) []os.FileInfo

func ReadDirFiltered(folder string, fn filterFiles) ([]os.FileInfo, error) {
Expand Down Expand Up @@ -189,22 +148,11 @@ func TrimSpace(value string) string {
}

func PrepareCommand(pattern string) (*exec.Cmd, error) {
parts, err := ParseCommandLine(pattern)
parts, err := properties.SplitQuotedString(pattern, `"'`, false)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this change the behaviour of the compile command --build-property and --build-properties flags?

Copy link
Member Author

Choose a reason for hiding this comment

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

No it should work exactly the same. This is code duplication that has never been removed.
(BTW a quick test would not be bad :-))

if err != nil {
return nil, errors.WithStack(err)
}
command := parts[0]
parts = parts[1:]
var args []string
for _, part := range parts {
if part == "" {
continue
}
args = append(args, part)
}

cmd := exec.Command(command, args...)
return cmd, nil
return exec.Command(parts[0], parts[1:]...), nil
}

func printableArgument(arg string) string {
Expand Down