Add support for the multiple code sources#10
Conversation
Vladyslav-Kuksiuk
requested review from
alexander-yevsyukov and
dmytro-kashcheiev
|
Can we have it as The reason for this is to clearly separate straight paths with those having a named prefix. |
alexander-yevsyukov
left a comment
alexander-yevsyukov
left a comment
There was a problem hiding this comment.
Please see my comments and questions.
| for _, path := range paths { | ||
| isPathSet, err := validatePathSet(path.Path) | ||
| if err != nil { | ||
| return true, fmt.Errorf("the given path `%s` does not exist", path) |
There was a problem hiding this comment.
Why do we have these error messages starting from lowercase and not ending with a period?
Clearly, these text are English sentences and should be formatted as such.
There was a problem hiding this comment.
It is a Go best practice: https://tip.golang.org/wiki/Errors
Even IntelliJ IDEA highlights errors that start with a capital letter or end with a period as warnings.
cli/cli_validation.go
Outdated
| } | ||
| if strings.ContainsAny(path.Name, ` *?:"<>|`) { | ||
| return true, fmt.Errorf("the given code path name `%s` "+ | ||
| "is not a valid name for the folder", path.Name) |
There was a problem hiding this comment.
Please tell why it is not a valid path name. Otherwise people would have to guess.
| var warnLines []string | ||
| for name, ps := range nameDuplicates { | ||
| if len(ps) > 1 { | ||
| warnLines = append(warnLines, "- "+name) |
There was a problem hiding this comment.
Does Go have an auto-formatting linter? The reason for asking is that we don't have space characters around + here and below. But we do in other places.
There was a problem hiding this comment.
As I understand it, IntelliJ IDEA uses gofmt by default, and this behavior is an issue: golang/go#34426.
I can add spaces, but IDEA will remove them during reformatting.
@alexander-yevsyukov Done. |
dmytro-kashcheiev
left a comment
dmytro-kashcheiev
left a comment
There was a problem hiding this comment.
@Vladyslav-Kuksiuk LGTM wtih minor comments.
3 participants
This PR provides support for specifying multiple code sources for the embedding with the following config syntax:
It is still possible to specify a single
code-path, as before, for backward compatibility.Note: When named paths are used, it is necessary to specify their names in the embedding instructions:
Do not forget the dollar sign (
$) before the path name.Resolves this issue.