Comments cleanup (#381)

cli/options.go

@@ -121,7 +121,7 @@ func applySourceOptions(cmd *cobra.Command, src *source.Source) error {
 		defaultOpts = options.Options{}
 	}
 
-	// FIXME: This should only apply source options?
+	// REVISIT: This should only apply source options?
 	flagOpts, err := getOptionsFromFlags(cmd.Flags(), ru.OptionsRegistry)
 	if err != nil {
 		return err

cli/output/format/format.go

@@ -17,8 +17,7 @@ func (f *Format) UnmarshalText(text []byte) error {
 		CSV, TSV, YAML:
 	case "table":
 		// Legacy: the "text" format used to be named "table".
-		// text = []byte(Text)
-		// FIXME: Probably should return an error now?
+		text = []byte(Text)
 	}
 
 	*f = Format(text)

drivers/json/ingest_json.go

@@ -18,7 +18,8 @@ import (
 )
 
 // DetectJSON returns a files.TypeDetectFunc that can detect JSON.
-func DetectJSON(sampleSize int) files.TypeDetectFunc { // FIXME: is DetectJSON actually working?
+func DetectJSON(sampleSize int) files.TypeDetectFunc {
+	// REVISIT: is DetectJSON actually working properly? Needs more tests.
 	return func(ctx context.Context, newRdrFn files.NewReaderFunc) (detected drivertype.Type, score float32,
 		err error,
 	) {

libsq/ast/render/expr.go

@@ -64,7 +64,7 @@ func doExpr(rc *Context, expr *ast.ExprNode) (string, error) {
 			}
 			sb.WriteString(val)
 		default:
-			// FIXME: Should log a warning here
+			// REVISIT: Should log a warning here?
 			// Shouldn't happen? Need to investigate.
 			sb.WriteString(child.Text())
 		}

libsq/ast/render/selectcols.go

@@ -41,7 +41,7 @@ func doSelectCols(rc *Context, cols []ast.ResultColumn) (string, error) {
 				return "", err
 			}
 		default:
-			// FIXME: We should be exhaustively checking the cases.
+			// REVISIT: We should be exhaustively checking the cases.
 			// Actually, this should probably be an error?
 			vals[i] = col.Text() // for now, we just return the raw text
 		}

libsq/core/kind/detect.go

@@ -175,7 +175,7 @@ func (d *Detector) doSampleString(s string) {
 					return nil, errz.Err(err)
 				}
 
-				// FIXME: Should time always return the canonical format?
+				// REVISIT: Should time always return the canonical format?
 				// return t.Format(format), nil
 				return t.Format(time.TimeOnly), nil
 			}

libsq/files/detect.go

@@ -65,7 +65,7 @@ func (fs *Files) DetectType(ctx context.Context, handle, loc string) (drivertype
 		}
 	}
 
-	// FIXME: We really should try to be smarter here, esp with sqlite files.
+	// REVISIT: We really should try to be smarter here, esp with sqlite files.
 
 	fs.mu.Lock()
 	defer fs.mu.Unlock()

libsq/source/source.go

@@ -18,9 +18,10 @@ const (
 	StdinHandle = "@stdin"
 
 	// ActiveHandle is the reserved handle for the active source.
-	// FIXME: it should be possible to use "@0" as the active handle, but
-	//  the SLQ grammar doesn't currently allow it. Possibly change this
-	//  value to "@0" after modifying the SLQ grammar.
+	//
+	// TODO: it should be possible to use "@0" as the active handle, but
+	// the SLQ grammar doesn't currently allow it. Possibly change this
+	// value to "@0" after modifying the SLQ grammar.
 	ActiveHandle = "@active"
 
 	// ScratchHandle is the reserved handle for the scratch source.
@@ -139,7 +140,7 @@ func (s *Source) ShortLocation() string {
 // Group returns the source's group. If s is in the root group,
 // the empty string is returned.
 //
-// FIXME: For root group, should "/" be returned instead of empty string?
+// REVISIT: For root group, should "/" be returned instead of empty string?
 func (s *Source) Group() string {
 	return groupFromHandle(s.Handle)
 }

testh/tu/tu.go

@@ -376,47 +376,6 @@ func MustStat(tb testing.TB, fp string) os.FileInfo {
 	return fi
 }
 
-// MustDrain drains r, failing t on error. If arg cloze is true,
-// r is closed if it's an io.Closer, even if the drain fails.
-// FIXME: delete this func.
-func MustDrain(tb testing.TB, r io.Reader, cloze bool) {
-	tb.Helper()
-	_, cpErr := io.Copy(io.Discard, r)
-	if !cloze {
-		require.NoError(tb, cpErr)
-		return
-	}
-
-	var closeErr error
-	if rc, ok := r.(io.Closer); ok {
-		closeErr = rc.Close()
-	}
-
-	require.NoError(tb, cpErr)
-	require.NoError(tb, closeErr)
-}
-
-// MustDrainN is like MustDrain, but also reports the number of bytes
-// drained. If arg cloze is true, r is closed if it's an io.Closer,
-// even if the drain fails.
-func MustDrainN(tb testing.TB, r io.Reader, cloze bool) int {
-	tb.Helper()
-	n, cpErr := io.Copy(io.Discard, r)
-	if !cloze {
-		require.NoError(tb, cpErr)
-		return int(n)
-	}
-
-	var closeErr error
-	if rc, ok := r.(io.Closer); ok {
-		closeErr = rc.Close()
-	}
-
-	require.NoError(tb, cpErr)
-	require.NoError(tb, closeErr)
-	return int(n)
-}
-
 // TempDir is the standard means for obtaining a temp dir for tests.
 // If arg clean is true, the temp dir is created via t.TempDir, and
 // thus is deleted on test cleanup.