o errors

- documented
    - clarified when and why panic() should be called
    - changed formatting verbs in error messages

o targets
    - removed panic()

debugger/targets.go

@@ -34,10 +34,6 @@ func (trg genericTarget) Label() string {
 func (trg genericTarget) CurrentValue() interface{} {
 	switch v := trg.currentValue.(type) {
 	case func() interface{}:
-		switch v := v().(type) {
-		case error:
-			panic(v)
-		}
 		return v()
 	default:
 		return v
@@ -49,11 +45,9 @@ func (trg genericTarget) FormatValue(val interface{}) string {
 	case string:
 		return val.(string)
 	case func() interface{}:
-		switch v := t().(type) {
+		switch t().(type) {
 		case string:
 			return val.(string)
-		case error:
-			panic(v)
 		default:
 			return fmt.Sprintf("%#v", val)
 		}

errors/doc.go

@@ -0,0 +1,44 @@
+// Package errors is a helper package for the error type. It defines the
+// AtariError type, a implementation of error the error interface, that allows
+// code to wrap errors around other errors and to allow normalised formatted
+// output of error messages.
+//
+// It also contains ID values for different classes of error and the English
+// messages for those error classes. These could be translated to other
+// languages, althought the i8n mechanism is not currently in place.
+//
+// The most useful feature is deduplication of wrapped errors. This means that
+// code does not need to worry about the immediate context of the function
+// which creates the error. For instance:
+//
+//	func main() { err := A() if err != nil { fmt.Println(err) } }
+//
+//	func A() error { err := B() if err != nil { return
+//	errors.New(errors.DebuggerError, err) } return nil }
+//
+//	func B() error { err := C() if err != nil { return
+//	errors.New(errors.DebuggerError, rr) } return nil }
+//
+//	func C() error { return errors.New(errors.PanicError, "C()", "not yet
+//	implemented") }
+//
+// If we follow the code from main() we can see that first error created is a
+// PanicError, wrapped in a DebuggerError, wrapped in a DebuggerError. The
+// message for the returned error to main() will be:
+//
+//	error debugging vcs: panic: C(): not yet implemented
+//
+// and not
+//
+//	error debugging vcs: error debugging vcs: panic: C(): not yet implemented
+//
+// As can be seen,  the error message has been deduplicatd, or normalised.
+//
+// The PanicError, used in the above example, is a special error that should be
+// used when something has happened such that the state of the emulation (or
+// the tool) can no longer be guaranteed.
+//
+// Actual panics should only be used when the error is so terrible that there i
+// nothing sensible to be done; useful for brute-enforcement of programming
+// constraints and in init() functions.
+package errors

errors/errors.go

@@ -11,14 +11,14 @@ type Errno int
 // Values is the type used to specify arguments for FormattedErrors
 type Values []interface{}
 
-// AtariError provides a convenient way of providing arguments to a
-// predefined error
+// AtariError allows code to specify a predefined error and not worry too much about the
+// message behind that error and how the message will be formatted on output.
 type AtariError struct {
 	Errno  Errno
 	Values Values
 }
 
-// New is used to create a new instance of a FormattedError
+// New is used to create a new instance of an AtariError.
 func New(errno Errno, values ...interface{}) AtariError {
 	return AtariError{
 		Errno:  errno,
@@ -26,6 +26,8 @@ func New(errno Errno, values ...interface{}) AtariError {
 	}
 }
 
+// Error returns the normalised error message. Most usefully, it compresses
+// duplicate adjacent AtariError instances.
 func (er AtariError) Error() string {
 	s := fmt.Sprintf(messages[er.Errno], er.Values...)
 
@@ -38,7 +40,7 @@ func (er AtariError) Error() string {
 	return strings.Join(p, ": ")
 }
 
-// Is checks if most recently wrapped error is a AtariError with a specific errno
+// Is checks if most recently wrapped error is an AtariError with a specific errno
 func Is(err error, errno Errno) bool {
 	switch er := err.(type) {
 	case AtariError:
@@ -47,7 +49,7 @@ func Is(err error, errno Errno) bool {
 	return false
 }
 
-// IsAny checks if most recently wrapped error is a AtariError with any errno
+// IsAny checks if most recently wrapped error is an AtariError, with any errno
 func IsAny(err error) bool {
 	switch err.(type) {
 	case AtariError:
@@ -56,7 +58,7 @@ func IsAny(err error) bool {
 	return false
 }
 
-// Has checks to see if the specified AtariError appears somewhere in the
+// Has checks to see if the specified AtariError errno appears somewhere in the
 // sequence of wrapped errors
 func Has(err error, errno Errno) bool {
 	if Is(err, errno) {

errors/messages.go

@@ -2,99 +2,99 @@ package errors
 
 var messages = map[Errno]string{
 	// panics
-	PanicError: "FATALITY: %s: %s",
+	PanicError: "panic: %v: %v",
 
 	// sentinals
 	UserInterrupt:   "user interrupt",
 	UserSuspend:     "user suspend",
-	ScriptEnd:       "end of script (%s)",
+	ScriptEnd:       "end of script (%v)",
 	PowerOff:        "emulated machine has been powered off",
-	PeriphUnplugged: "controller unplugged from '%s'",
-	TVOutOfSpec:     "tv out of spec: %s",
+	PeriphUnplugged: "controller unplugged from %v",
+	TVOutOfSpec:     "tv out of spec: %v",
 
 	// program modes
-	PlayError:        "error emulating vcs: %s",
-	DebuggerError:    "error debugging vcs: %s",
-	PerformanceError: "error during performance profiling: %s",
-	DisasmError:      "error during disassembly: %s",
+	PlayError:        "error emulating vcs: %v",
+	DebuggerError:    "error debugging vcs: %v",
+	PerformanceError: "error during performance profiling: %v",
+	DisasmError:      "error during disassembly: %v",
 
 	// debugger
-	ParserError:          "parser error: %s: %s (char %d)", // first placeholder is the command definition
-	ValidationError:      "%s for %s",
-	InvalidTarget:        "invalid target (%s)",
-	CommandError:         "%s",
-	TerminalError:        "%s",
+	ParserError:          "parser error: %v: %v (char %d)", // first placeholder is the command definition
+	ValidationError:      "%v for %v",
+	InvalidTarget:        "invalid target (%v)",
+	CommandError:         "%v",
+	TerminalError:        "%v",
 	GUIEventError:        "%v",
 	ReflectionNotRunning: "reflection process is not running",
 
 	// script
-	ScriptFileError:       "script error: %s",
-	ScriptFileUnavailable: "script error: cannot open script file (%s)",
-	ScriptRunError:        "script error: use of '%s' is not allowed in scripts [%s::%d]",
-	ScriptScribeError:     "script scribe error: %s",
+	ScriptFileError:       "script error: %v",
+	ScriptFileUnavailable: "script error: cannot open script file (%v)",
+	ScriptRunError:        "script error: use of '%v' is not allowed in scripts [%v::%d]",
+	ScriptScribeError:     "script scribe error: %v",
 
 	// recorder
-	RecordingError:    "controller recording error: %s",
-	PlaybackError:     "controller playback error: %s",
-	PlaybackHashError: "controller playback error: hash error: %s",
+	RecordingError:    "controller recording error: %v",
+	PlaybackError:     "controller playback error: %v",
+	PlaybackHashError: "controller playback error: hash error: %v",
 
 	// database
-	DatabaseError:           "database error: %s",
+	DatabaseError:           "database error: %v",
 	DatabaseSelectEmpty:     "database error: no selected entries",
 	DatabaseKeyError:        "database error: no such key in database [%v]",
-	DatabaseFileUnavailable: "database error: cannot open database (%s)",
+	DatabaseFileUnavailable: "database error: cannot open database (%v)",
 
 	// regression
-	RegressionError:         "regression test error: %s",
-	RegressionFrameError:    "regression test error: frame entry: %s",
-	RegressionPlaybackError: "regression test error: playback entry: %s",
+	RegressionError:         "regression test error: %v",
+	RegressionFrameError:    "regression test error: frame entry: %v",
+	RegressionPlaybackError: "regression test error: playback entry: %v",
 
 	// setup
-	SetupError:      "setup error: %s",
-	SetupPanelError: "setup error: panel entry: %s",
+	SetupError:      "setup error: %v",
+	SetupPanelError: "setup error: panel entry: %v",
 
 	// symbols
-	SymbolsFileError:       "symbols error: error processing symbols file: %s",
-	SymbolsFileUnavailable: "symbols error: no symbols file for %s",
-	SymbolUnknown:          "symbols error: unrecognised symbol (%s)",
+	SymbolsFileError:       "symbols error: error processing symbols file: %v",
+	SymbolsFileUnavailable: "symbols error: no symbols file for %v",
+	SymbolUnknown:          "symbols error: unrecognised symbol (%v)",
 
 	// cartridgeloader
-	CartridgeLoader: "cartridge loading error: %s",
+	CartridgeLoader: "cartridge loading error: %v",
 
 	// vcs
-	VCSError: "vcs error: %s",
+	VCSError: "vcs error: %v",
 
 	// cpu
-	UnimplementedInstruction:       "cpu error: unimplemented instruction (%0#x) at (%#04x)",
+	UnimplementedInstruction:       "cpu error: unimplemented instruction (%#02x) at (%#04x)",
 	InvalidOpcode:                  "cpu error: invalid opcode (%#04x)",
-	InvalidResult:                  "cpu error: %s",
+	InvalidResult:                  "cpu error: %v",
 	ProgramCounterCycled:           "cpu error: program counter cycled back to 0x0000",
-	InvalidOperationMidInstruction: "cpu error: invalid operation mid-instruction (%s)",
+	InvalidOperationMidInstruction: "cpu error: invalid operation mid-instruction (%v)",
 
 	// memory
-	MemoryError:         "memory error: %s",
+	MemoryError:         "memory error: %v",
 	UnreadableAddress:   "memory error: memory location is not readable (%#04x)",
 	UnwritableAddress:   "memory error: memory location is not writable (%#04x)",
-	UnpokeableAddress:   "memory error: cannot poke address (%v)",
-	UnpeekableAddress:   "memory error: cannot peek address (%v)",
-	UnrecognisedAddress: "memory error: address unrecognised (%v)",
+	UnpokeableAddress:   "memory error: cannot poke address (%#04x)",
+	UnpeekableAddress:   "memory error: cannot peek address (%#04x)",
+	UnrecognisedAddress: "memory error: address unrecognised (%#04x)",
 
 	// cartridges
-	CartridgeError:   "cartridge error: %s",
+	CartridgeError:   "cartridge error: %v",
 	CartridgeEjected: "cartridge error: no cartridge attached",
 
 	// peripherals
-	PeriphHardwareUnavailable: "peripheral error: controller hardware unavailable (%s)",
-	UnknownPeriphEvent:        "peripheral error: %s: unsupported event (%v)",
+	PeriphHardwareUnavailable: "peripheral error: controller hardware unavailable (%v)",
+	UnknownPeriphEvent:        "peripheral error: %v: unsupported event (%v)",
 
 	// television
 	UnknownTVRequest: "television error: unsupported request (%v)",
-	Television:       "television error: %s",
+	Television:       "television error: %v",
 
 	// screen digest
-	ScreenDigest: "television error: screendigest: %s",
+	ScreenDigest: "television error: screendigest: %v",
 
 	// gui
 	UnsupportedGUIRequest: "gui error: unsupported request (%v)",
-	SDL:                   "gui error: SDL: %s",
+	SDL:                   "gui error: SDL: %v",
 }

hardware/tia/video/collisions.go

@@ -63,6 +63,11 @@ func (col *collisions) setMemory(collisionAddress uint16) {
 	case addresses.CXPPMM:
 		col.mem.ChipWrite(addresses.CXPPMM, col.cxppmm)
 	default:
+		// it would be nice to get rid of this panic() but it's doing no harm
+		// and returning an error from here would be ugly.
+		//
+		// Best solution is to figure out how to constrain memory addresses by
+		// type...
 		panic(fmt.Sprintf("not a collision address (%04x)", collisionAddress))
 	}
 }

paths/doc.go

@@ -0,0 +1,7 @@
+// Package paths should be used whenever a request to the filesystem is
+// made. The functions herein make sure that the correct path (depending on the
+// operating system being targeted) is used for the resource.
+//
+// Because this package handles project specific details it should be used
+// instead of the Go standard path package
+package paths

paths/posix.go

@@ -1,12 +1,11 @@
 //+build darwin dragonfly freebsd linux openbsd netbsd solaris
 
-// The paths package should be used whenever a request to the filesystem is
+// Package paths should be used whenever a request to the filesystem is
 // made. The functions herein make sure that the correct path (depending on the
 // operating system being targeted) is used for the resource.
 //
 // Because this package handles project specific details it should be used
 // instead of the Go standard path package
-
 package paths
 
 import (