Skip to content

Commit

Permalink
cmd: Add more descriptive errors to syntax errors and make them red.
Browse files Browse the repository at this point in the history
  • Loading branch information
Sandertv committed Dec 24, 2024
1 parent e920e59 commit c6641ba
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 48 deletions.
62 changes: 31 additions & 31 deletions server/cmd/argument.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
package cmd

import (
"errors"
"fmt"
"github.com/df-mc/dragonfly/server/internal/sliceutil"
"github.com/df-mc/dragonfly/server/player/chat"
"github.com/df-mc/dragonfly/server/world"
"github.com/go-gl/mathgl/mgl64"
"math/rand"
Expand All @@ -21,18 +19,24 @@ type Line struct {
args []string
seen []string
src Source
cmd Command
}

// SyntaxError returns a translated syntax error.
func (line *Line) SyntaxError() error {
if len(line.args) == 0 {
return chat.MessageCommandSyntax.F(line.seen, "", "")
return MessageSyntax.F(strings.Join(line.seen, " "), "", "")
}
next := strings.Join(line.args[1:], " ")
if next != "" {
next = " " + next
}
return chat.MessageCommandSyntax.F(strings.Join(line.seen, " ")+" ", line.args[0], next)
return MessageSyntax.F(strings.Join(line.seen, " ")+" ", line.args[0], next)
}

// UsageError returns a translated usage error.
func (line *Line) UsageError() error {
return MessageUsage.F(line.cmd.Usage())
}

// Next reads the next argument from the command line and returns it. If there were no more arguments to
Expand Down Expand Up @@ -137,19 +141,15 @@ func (p parser) parseArgument(line *Line, v reflect.Value, optional bool, name s
return err, err == nil
}

// ErrInsufficientArgs is returned by argument parsing functions if it does not have sufficient arguments
// passed and is not optional.
var ErrInsufficientArgs = errors.New("not enough arguments for command")

// int ...
func (p parser) int(line *Line, v reflect.Value) error {
arg, ok := line.Next()
if !ok {
return line.SyntaxError()
return line.UsageError()
}
value, err := strconv.ParseInt(arg, 10, v.Type().Bits())
if err != nil {
return line.SyntaxError()
return MessageNumberInvalid.F(arg)
}
v.SetInt(value)
return nil
Expand All @@ -159,11 +159,11 @@ func (p parser) int(line *Line, v reflect.Value) error {
func (p parser) uint(line *Line, v reflect.Value) error {
arg, ok := line.Next()
if !ok {
return line.SyntaxError()
return line.UsageError()
}
value, err := strconv.ParseUint(arg, 10, v.Type().Bits())
if err != nil {
return line.SyntaxError()
return MessageNumberInvalid.F(arg)
}
v.SetUint(value)
return nil
Expand All @@ -173,11 +173,11 @@ func (p parser) uint(line *Line, v reflect.Value) error {
func (p parser) float(line *Line, v reflect.Value) error {
arg, ok := line.Next()
if !ok {
return line.SyntaxError()
return line.UsageError()
}
value, err := strconv.ParseFloat(arg, v.Type().Bits())
if err != nil {
return line.SyntaxError()
return MessageNumberInvalid.F(arg)
}
v.SetFloat(value)
return nil
Expand All @@ -187,7 +187,7 @@ func (p parser) float(line *Line, v reflect.Value) error {
func (p parser) string(line *Line, v reflect.Value) error {
arg, ok := line.Next()
if !ok {
return line.SyntaxError()
return line.UsageError()
}
v.SetString(arg)
return nil
Expand All @@ -197,11 +197,11 @@ func (p parser) string(line *Line, v reflect.Value) error {
func (p parser) bool(line *Line, v reflect.Value) error {
arg, ok := line.Next()
if !ok {
return line.SyntaxError()
return line.UsageError()
}
value, err := strconv.ParseBool(arg)
if err != nil {
return line.SyntaxError()
return MessageBooleanInvalid.F(arg)
}
v.SetBool(value)
return nil
Expand All @@ -211,14 +211,14 @@ func (p parser) bool(line *Line, v reflect.Value) error {
func (p parser) enum(line *Line, val reflect.Value, v Enum, source Source) error {
arg, ok := line.Next()
if !ok {
return line.SyntaxError()
return line.UsageError()
}
opts := v.Options(source)
ind := slices.IndexFunc(opts, func(s string) bool {
return strings.EqualFold(s, arg)
})
if ind < 0 {
return line.SyntaxError()
return MessageParameterInvalid.F(arg)
}
val.SetString(opts[ind])
return nil
Expand All @@ -228,12 +228,12 @@ func (p parser) enum(line *Line, val reflect.Value, v Enum, source Source) error
func (p parser) sub(line *Line, name string) error {
arg, ok := line.Next()
if !ok {
return line.SyntaxError()
return line.UsageError()
}
if strings.EqualFold(name, arg) {
return nil
}
return line.SyntaxError()
return MessageParameterInvalid.F(arg)
}

// vec3 ...
Expand Down Expand Up @@ -262,7 +262,7 @@ func (p parser) targets(line *Line, v reflect.Value, tx *world.Tx) error {
return err
}
if len(targets) == 0 {
return chat.MessageCommandNoTargets.F()
return MessageNoTargets.F()
}
v.Set(reflect.ValueOf(targets))
return nil
Expand All @@ -273,9 +273,9 @@ func (p parser) parseTargets(line *Line, tx *world.Tx) ([]Target, error) {
entities, players := targets(tx)
first, ok := line.Next()
if !ok {
return nil, line.SyntaxError()
return nil, line.UsageError()
}
switch first {
switch first[:2] {
case "@p":
pos := line.src.Position()
playerDistances := make([]float64, len(players))
Expand All @@ -301,27 +301,27 @@ func (p parser) parseTargets(line *Line, tx *world.Tx) ([]Target, error) {
}
return []Target{players[rand.Intn(len(players))]}, nil
default:
target, ok := p.parsePlayer(line, players)
if ok {
return []Target{target}, nil
target, err := p.parsePlayer(line, players)
if err != nil {
return nil, err
}
return nil, nil
return []Target{target}, nil
}
}

// parsePlayer parses one Player from the Line, consuming multiple arguments
// from Line if necessary.
func (p parser) parsePlayer(line *Line, players []NamedTarget) (Target, bool) {
func (p parser) parsePlayer(line *Line, players []NamedTarget) (Target, error) {
name := ""
for i := 0; i < line.Len(); i++ {
name += line.args[0]
if ind := slices.IndexFunc(players, func(target NamedTarget) bool {
return strings.EqualFold(target.Name(), name)
}); ind != -1 {
return players[ind], true
return players[ind], nil
}
name += " "
line.RemoveNext()
}
return nil, false
return nil, MessagePlayerNotFound.F()
}
22 changes: 12 additions & 10 deletions server/cmd/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package cmd
import (
"encoding/csv"
"fmt"
"github.com/df-mc/dragonfly/server/player/chat"
"github.com/df-mc/dragonfly/server/world"
"go/ast"
"reflect"
Expand Down Expand Up @@ -131,7 +130,7 @@ func (cmd Command) Execute(args string, source Source, tx *world.Tx) {
defer source.SendCommandOutput(output)

var leastErroneous error
leastArgsLeft := len(strings.Split(args, " "))
var leastArgsLeft *Line

for _, v := range cmd.v {
cp := reflect.New(v.Type())
Expand All @@ -150,15 +149,18 @@ func (cmd Command) Execute(args string, source Source, tx *world.Tx) {
}
continue
}
if line.Len() <= leastArgsLeft {
if leastArgsLeft == nil || line.Len() <= leastArgsLeft.Len() {
// If the line had less (or equal) arguments left than the previous lowest, we update the error,
// so that we can return an error that applies for the most successful Runnable.
leastErroneous = err
leastArgsLeft = line.Len()
leastArgsLeft = line
}
}
// No working Runnable found for the arguments passed. We add the most applicable error to the output and
// stop there.
// No working Runnable found for the arguments passed. We add the most
// applicable error to the output and stop there.
if leastArgsLeft != nil {
output.Error(leastArgsLeft.SyntaxError())
}
output.Error(leastErroneous)
}

Expand Down Expand Up @@ -222,7 +224,7 @@ func (cmd Command) String() string {
// leftover command line.
func (cmd Command) executeRunnable(v reflect.Value, args string, source Source, output *Output, tx *world.Tx) (*Line, error) {
if a, ok := v.Interface().(Allower); ok && !a.Allow(source) {
return nil, chat.MessageCommandUnknown.F(cmd.name)
return nil, MessageUnknown.F(cmd.name)
}

var argFrags []string
Expand All @@ -234,12 +236,12 @@ func (cmd Command) executeRunnable(v reflect.Value, args string, source Source,
// When LazyQuotes is enabled, this really never appears to return
// an error when we read only one line. Just in case it does though,
// we return the command usage.
return nil, chat.MessageCommandUsage.F(cmd.Usage())
return nil, MessageUsage.F(cmd.Usage())
}
argFrags = record
}
parser := parser{}
arguments := &Line{args: argFrags, src: source, seen: []string{"/" + cmd.name}}
arguments := &Line{args: argFrags, src: source, seen: []string{"/" + cmd.name}, cmd: cmd}

// We iterate over all the fields of the struct: Each of the fields will have an argument parsed to
// produce its value.
Expand All @@ -265,7 +267,7 @@ func (cmd Command) executeRunnable(v reflect.Value, args string, source Source,
}
}
if arguments.Len() != 0 {
return arguments, arguments.SyntaxError()
return arguments, arguments.UsageError()
}

v.Interface().(Runnable).Run(source, output, tx)
Expand Down
15 changes: 15 additions & 0 deletions server/cmd/output.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"errors"
"fmt"
"github.com/df-mc/dragonfly/server/player/chat"
"golang.org/x/text/language"
)

// Output holds the output of a command execution. It holds success messages
Expand Down Expand Up @@ -78,3 +79,17 @@ func (o *Output) MessageCount() int {
type stringer string

func (s stringer) String() string { return string(s) }

var MessageSyntax = chat.Translate(str("%commands.generic.syntax"), 3, `Syntax error: unexpected value: at "%v>>%v<<%v"`).Enc("<red>%v</red>")
var MessageUsage = chat.Translate(str("%commands.generic.usage"), 1, `Usage: %v`).Enc("<red>%v</red>")
var MessageUnknown = chat.Translate(str("%commands.generic.unknown"), 1, `Unknown command: "%v": Please check that the command exists and that you have permission to use it.`).Enc("<red>%v</red>")
var MessageNoTargets = chat.Translate(str("%commands.generic.noTargetMatch"), 0, `No targets matched selector`).Enc("<red>%v</red>")
var MessageNumberInvalid = chat.Translate(str("%commands.generic.num.invalid"), 1, `'%v' is not a valid number`).Enc("<red>> %v</red>")
var MessageBooleanInvalid = chat.Translate(str("%commands.generic.boolean.invalid"), 1, `'%v' is not true or false`).Enc("<red>> %v</red>")
var MessagePlayerNotFound = chat.Translate(str("%commands.generic.player.notFound"), 0, `That player cannot be found`).Enc("<red>> %v</red>")
var MessageParameterInvalid = chat.Translate(str("%commands.generic.parameter.invalid"), 1, `'%v' is not a valid parameter`).Enc("<red>> %v</red>")

type str string

// Resolve returns the translation identifier as a string.
func (s str) Resolve(language.Tag) string { return string(s) }
5 changes: 0 additions & 5 deletions server/player/chat/translate.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,6 @@ var MessageJoin = Translate(str("%multiplayer.player.joined"), 1, `%v joined the
var MessageQuit = Translate(str("%multiplayer.player.left"), 1, `%v left the game`).Enc("<yellow>%v</yellow>")
var MessageServerDisconnect = Translate(str("%disconnect.disconnected"), 0, `Disconnected by Server`).Enc("<yellow>%v</yellow>")

var MessageCommandSyntax = Translate(str("%commands.generic.syntax"), 3, `Syntax error: unexpected value: at "%v>>%v<<%v"`)
var MessageCommandUsage = Translate(str("%commands.generic.usage"), 1, `Usage: %v`)
var MessageCommandUnknown = Translate(str("%commands.generic.unknown"), 1, `Unknown command: "%v": Please check that the command exists and that you have permission to use it.`)
var MessageCommandNoTargets = Translate(str("%commands.generic.noTargetMatch"), 0, `No targets matched selector`)

type str string

// Resolve returns the translation identifier as a string.
Expand Down
2 changes: 1 addition & 1 deletion server/player/player.go
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ func (p *Player) ExecuteCommand(commandLine string) {
command, ok := cmd.ByAlias(args[0][1:])
if !ok {
o := &cmd.Output{}
o.Errort(chat.MessageCommandUnknown, args[0])
o.Errort(cmd.MessageUnknown, args[0])
p.SendCommandOutput(o)
return
}
Expand Down
2 changes: 1 addition & 1 deletion server/world/entity.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func (e *EntityHandle) Close() error {
// run the transaction function once it is. If the Entity is closed before
// ExecWorld is called, ExecWorld will return false immediately without running
// the transaction function.
func (e *EntityHandle) ExecWorld(f func(tx *Tx, e Entity)) (run bool) {
func (e *EntityHandle) ExecWorld(f func(tx *Tx, e Entity)) bool {
return e.execWorld(f, false)
}

Expand Down

0 comments on commit c6641ba

Please sign in to comment.