hotfix for ignored --env flag issue (#269)

.github/workflows/tests.yml

@@ -12,7 +12,6 @@ jobs:
     strategy:
       matrix:
         go-version:
-          - "1.16.x"
           - "1.17.x"
           - "1.18.x"
 
@@ -56,7 +55,6 @@ jobs:
     strategy:
       matrix:
         go-version:
-          - "1.16.x"
           - "1.17.x"
           - "1.18.x"
 
@@ -102,7 +100,6 @@ jobs:
     strategy:
       matrix:
         go-version:
-          - "1.16.x"
           - "1.17.x"
           - "1.18.x"
 
@@ -146,7 +143,6 @@ jobs:
     strategy:
       matrix:
         go-version:
-          - "1.16.x"
           - "1.17.x"
           - "1.18.x"
 
@@ -187,7 +183,6 @@ jobs:
     strategy:
       matrix:
         go-version:
-          - "1.16.x"
           - "1.17.x"
           - "1.18.x"
         os:
@@ -213,7 +208,15 @@ jobs:
         shell: bash
 
       - name: Test
+        if: ${{ matrix.os != 'windows-latest' }}
         env:
           SODA_DIALECT: "sqlite"
         run: |
           go test -tags sqlite -race -cover ./...
+
+      - name: Short Test
+        if: ${{ matrix.os == 'windows-latest' }}
+        env:
+          SODA_DIALECT: "sqlite"
+        run: |
+          go test -tags sqlite ./...

pop.go

@@ -56,6 +56,12 @@ func DialectSupported(d string) bool {
 	return false
 }
 
+// CanonicalDialect checks if the given synonym (could be a valid dialect too)
+// is a registered synonym and returns the canonical dialect for the synonym.
+// Otherwise, it returns the lowercase value of the given synonym.
+//
+// Note that it does not check if the return value is a valid (supported)
+// dialect so you need to check it with `DialectSupported()`.
 func CanonicalDialect(synonym string) string {
 	d := strings.ToLower(synonym)
 	if syn, ok := dialectSynonyms[d]; ok {

soda/cmd/root.go

@@ -20,8 +20,22 @@ var RootCmd = &cobra.Command{
 	Short:        "A tasty treat for all your database needs",
 	PersistentPreRun: func(c *cobra.Command, args []string) {
 		fmt.Printf("pop %s\n\n", Version)
+
+		/* NOTE: Do not use c.PersistentFlags. `c` is not always the
+		RootCmd. The naming is confusing. The meaning of "persistent"
+		in the `PersistentPreRun` is something like "this function will
+		be 'sticky' to all subcommands and will run for them. So `c`
+		can be any subcommands.
+		However, the meaning of "persistent" in the `PersistentFlags`
+		is, as the function comment said, "persistent FlagSet
+		specifically set in the **current command**" so it is sticky
+		to specific command!
+
+		Use c.Flags() or c.Root().Flags() here.
+		*/
+
 		// CLI flag has priority
-		if !c.PersistentFlags().Changed("env") {
+		if !c.Flags().Changed("env") {
 			env = defaults.String(os.Getenv("GO_ENV"), env)
 		}
 		// TODO! Only do this when the command needs it.

soda/cmd/root_integration_test.go

@@ -7,27 +7,41 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func Test_RootCmd_NoArg(t *testing.T) {
+func Test_RootCmd_Environment(t *testing.T) {
 	oldEnv := os.Getenv("GO_ENV")
 	defer os.Setenv("GO_ENV", oldEnv)
 
-	// Fallback on default env
 	r := require.New(t)
 	c := RootCmd
-	c.SetArgs([]string{})
+
+	// Fallback on default env
+	c.SetArgs([]string{"help"})
 	err := c.Execute()
 	r.NoError(err)
 	r.Equal("development", env)
 
 	// Override with GO_ENV
-	c.SetArgs([]string{})
+	c.SetArgs([]string{"help"})
 	os.Setenv("GO_ENV", "test")
 	err = c.Execute()
 	r.NoError(err)
 	r.Equal("test", env)
 
+	// CLI flag priority: the preferred order of flags and commands
+	c.SetArgs([]string{
+		"--env",
+		"production",
+		"help",
+	})
+	os.Setenv("GO_ENV", "test")
+	err = c.Execute()
+	r.NoError(err)
+	r.Equal("production", env)
+
+	// the following order works fine now but need to be considered again
 	// CLI flag priority
 	c.SetArgs([]string{
+		"help",
 		"--env",
 		"production",
 	})