cli: add hash to path on conflicting filename (#152)

* cli: add testcase for output with extension

* cli: add testcase for relative paths

* cli: add hash to filepath on duplicate

* cli: better error message when input points to folder

* cli: try to use filepath.ToSlash

* cli: print filepath

* cli: try another print filepath

* cli: try another print filepath for glob

* cli: try splitpattern with toslash

* try: remove fmt calls

* cli: try to remove ToSlash in hashing

* cli: re-introduce ToSlash in hashing

Author: JohannesKaufmann

cli/html2markdown/cmd/exec.go

@@ -163,6 +163,11 @@ func (cli *CLI) run() ([]error, error) {
 		return nil, err
 	}
 
+	err = calculateOutputPaths(cli.config.inputFilepath, inputs)
+	if err != nil {
+		return nil, err
+	}
+
 	for _, input := range inputs {
 		data, err := cli.readInput(input)
 		if err != nil {
@@ -174,7 +179,7 @@ func (cli *CLI) run() ([]error, error) {
 			return nil, err
 		}
 
-		err = cli.writeOutput(outputType, getOutputFileName(input.fullFilepath), markdown)
+		err = cli.writeOutput(outputType, input.outputFullFilepath, markdown)
 		if err != nil {
 			return nil, err
 		}

cli/html2markdown/cmd/exec_test.go

@@ -52,7 +52,7 @@ type CLIGoldenInput struct {
 	inputArgs  []string
 }
 
-func cliGoldenTester(t *testing.T, input CLIGoldenInput) {
+func cliGoldenTester(t *testing.T, folderpath string, input CLIGoldenInput) {
 	if input.modeStdin == modeTerminal && input.inputStdin != nil {
 		t.Fatal("invalid test: cannot provide stdin without pipe mode")
 	}
@@ -77,7 +77,7 @@ func cliGoldenTester(t *testing.T, input CLIGoldenInput) {
 		t.Fatal("neither stdout nor stderr have any content")
 	}
 
-	g := goldie.New(t)
+	g := goldie.New(t, goldie.WithFixtureDir(filepath.Join(folderpath, "testdata")))
 	g.Assert(t, filepath.Join(t.Name(), "stdout"), stdout.Bytes())
 	g.Assert(t, filepath.Join(t.Name(), "stderr"), stderr.Bytes())
 }
@@ -119,6 +119,14 @@ func TestExecute(t *testing.T) {
 	directoryPath := newTestDirWithFiles(t)
 	defer os.RemoveAll(directoryPath)
 
+	originalDir, err := os.Getwd()
+	if err != nil {
+		t.Fatal(err)
+	}
+	// By switching the directory we can work with relative paths.
+	// This makes it easier to test the output of error messages...
+	chdirWithCleanup(t, directoryPath)
+
 	testCases := []struct {
 		desc  string
 		input CLIGoldenInput
@@ -512,6 +520,19 @@ func TestExecute(t *testing.T) {
 				inputArgs:  []string{"html2markdown", "--input", "not_found.html"},
 			},
 		},
+		{
+			desc: "[files] input directory",
+
+			input: CLIGoldenInput{
+				modeStdin:  modeTerminal,
+				modeStdout: modePipe,
+				modeStderr: modePipe,
+
+				inputStdin: nil,
+				inputArgs:  []string{"html2markdown", "--input", "input"}, // <-- "input" is a folder
+			},
+		},
+
 		{
 			desc: "[files] multiple values",
 
@@ -539,7 +560,7 @@ func TestExecute(t *testing.T) {
 	}
 	for _, tC := range testCases {
 		t.Run(tC.desc, func(t *testing.T) {
-			cliGoldenTester(t, tC.input)
+			cliGoldenTester(t, originalDir, tC.input)
 		})
 	}
 }

cli/html2markdown/cmd/files_test.go

@@ -67,6 +67,27 @@ func newTestDirWithFiles(t *testing.T) string {
 	return directoryPath
 }
 
+// chdirWithCleanup changes the current working directory to the named directory,
+// and then restore the original working directory at the end of the test.
+//
+// TODO: Once we are on 1.24 we can replace this with t.Chdir()
+func chdirWithCleanup(t *testing.T, dir string) {
+	olddir, err := os.Getwd()
+	if err != nil {
+		t.Fatalf("chdir: %v", err)
+	}
+	if err := os.Chdir(dir); err != nil {
+		t.Fatalf("chdir %s: %v", dir, err)
+	}
+
+	t.Cleanup(func() {
+		if err := os.Chdir(olddir); err != nil {
+			t.Errorf("chdir to original working directory %s: %v", olddir, err)
+			os.Exit(1)
+		}
+	})
+}
+
 func writePipeChar(buf *bytes.Buffer, index int) {
 	if index == 0 {
 		return
@@ -306,10 +327,80 @@ func TestExecute_NotOverwrite(t *testing.T) {
 		expectRepresentation(t, directoryPath, `
 .
 ├─output.md "**file content A**"
-	`)
+		`)
 	})
 }
 
+func TestExecute_DuplicateFiles(t *testing.T) {
+	directoryPath := newTestDir(t)
+	defer os.RemoveAll(directoryPath)
+
+	chdirWithCleanup(t, directoryPath)
+
+	inputFolderA := filepath.Join(directoryPath, "input", "a")
+	inputFolderB := filepath.Join(directoryPath, "input", "b")
+	inputFolderC := filepath.Join(directoryPath, "input", "nested", "c")
+
+	err := os.MkdirAll(inputFolderA, os.ModePerm)
+	if err != nil {
+		t.Fatal(err)
+	}
+	err = os.MkdirAll(inputFolderB, os.ModePerm)
+	if err != nil {
+		t.Fatal(err)
+	}
+	err = os.MkdirAll(inputFolderC, os.ModePerm)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = os.WriteFile(filepath.Join(inputFolderA, "random.html"), []byte("file a"), 0644)
+	if err != nil {
+		t.Fatal(err)
+	}
+	err = os.WriteFile(filepath.Join(inputFolderB, "random.html"), []byte("file b"), 0644)
+	if err != nil {
+		t.Fatal(err)
+	}
+	err = os.WriteFile(filepath.Join(inputFolderC, "random.html"), []byte("file c"), 0644)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// - - - - - - - - - //
+	args := []string{"html2markdown", "--input", filepath.Join(".", "input", "**", "*"), "--output", filepath.Join(".", "output") + "/"}
+
+	stdin := &FakeFile{mode: modeTerminal}
+	stdout := &FakeFile{mode: modePipe}
+	stderr := &FakeFile{mode: modePipe}
+
+	Run(stdin, stdout, stderr, args, testRelease)
+
+	if len(stderr.Bytes()) != 0 {
+		t.Fatalf("expected no stderr content but got %q", stderr.String())
+	}
+	if len(stdout.Bytes()) != 0 {
+		t.Fatalf("expected no stdout content")
+	}
+	// - - - - - - - - - //
+
+	expectRepresentation(t, directoryPath, `
+.
+├─input
+│ ├─a
+│ │ ├─random.html "file a"
+│ ├─b
+│ │ ├─random.html "file b"
+│ ├─nested
+│ │ ├─c
+│ │ │ ├─random.html "file c"
+├─output
+│ ├─random.689330a60f.md "file b"
+│ ├─random.f679b6e0c2.md "file c"
+│ ├─random.md "file a"
+	`)
+}
+
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - //
 
 func TestExecute_FilePattern(t *testing.T) {
@@ -355,7 +446,27 @@ func TestExecute_FilePattern(t *testing.T) {
 ├─output
 │ ├─websites
 │ │ ├─the_cool_website.md "**file content A**"
+			`,
+		},
+		{
+			desc: "output to a specific extension",
+			assembleArgs: func(dir string) []string {
+				input := filepath.Join(dir, "input", "website_a.html")
+				output := filepath.Join(dir, "output", "websites", "the_cool_website.txt")
 
+				return []string{"html2markdown", "--input", input, "--output", output}
+			},
+			expected: `
+.
+├─input
+│ ├─nested
+│ │ ├─website_c.html "<i>file content C</i>"
+│ ├─random.txt "other random file"
+│ ├─website_a.html "<strong>file content A</strong>"
+│ ├─website_b.html "<strong>file content B</strong>"
+├─output
+│ ├─websites
+│ │ ├─the_cool_website.txt "**file content A**"
 			`,
 		},
 		{
@@ -379,6 +490,72 @@ func TestExecute_FilePattern(t *testing.T) {
 
 			`,
 		},
+
+		// - - - - - - - - - - - - relative path - - - - - - - - - - - - //
+		{
+			desc: "relative path: output to a specific file",
+			assembleArgs: func(_ string) []string {
+				input := filepath.Join(".", "input", "website_a.html")
+				output := filepath.Join(".", "output", "websites", "the_cool_website.md")
+
+				return []string{"html2markdown", "--input", input, "--output", output}
+			},
+			expected: `
+.
+├─input
+│ ├─nested
+│ │ ├─website_c.html "<i>file content C</i>"
+│ ├─random.txt "other random file"
+│ ├─website_a.html "<strong>file content A</strong>"
+│ ├─website_b.html "<strong>file content B</strong>"
+├─output
+│ ├─websites
+│ │ ├─the_cool_website.md "**file content A**"
+			`,
+		},
+		{
+			desc: "relative path: direct website html files",
+			assembleArgs: func(_ string) []string {
+				input := filepath.Join(".", "input", "website*.html")
+				output := filepath.Join(".", "output") + string(os.PathSeparator)
+
+				return []string{"html2markdown", "--input", input, "--output", output}
+			},
+			expected: `
+.
+├─input
+│ ├─nested
+│ │ ├─website_c.html "<i>file content C</i>"
+│ ├─random.txt "other random file"
+│ ├─website_a.html "<strong>file content A</strong>"
+│ ├─website_b.html "<strong>file content B</strong>"
+├─output
+│ ├─website_a.md "**file content A**"
+│ ├─website_b.md "**file content B**"
+			`,
+		},
+		{
+			desc: "relative path: output to current directory",
+			assembleArgs: func(_ string) []string {
+				input := filepath.Join(".", "input", "website*.html")
+				output := "." + string(os.PathSeparator)
+
+				return []string{"html2markdown", "--input", input, "--output", output}
+			},
+			expected: `
+.
+├─input
+│ ├─nested
+│ │ ├─website_c.html "<i>file content C</i>"
+│ ├─random.txt "other random file"
+│ ├─website_a.html "<strong>file content A</strong>"
+│ ├─website_b.html "<strong>file content B</strong>"
+├─output
+├─website_a.md "**file content A**"
+├─website_b.md "**file content B**"
+			`,
+		},
+
 		// - - - - - - - - - - - - pattern - - - - - - - - - - - - //
 		{
 			desc: "pattern matches single file",
@@ -522,6 +699,8 @@ func TestExecute_FilePattern(t *testing.T) {
 			directoryPath := newTestDirWithFiles(t)
 			defer os.RemoveAll(directoryPath)
 
+			chdirWithCleanup(t, directoryPath)
+
 			args := tC.assembleArgs(directoryPath)
 			t.Logf("args: %+v\n", args)
 

cli/html2markdown/cmd/io_input.go

@@ -11,21 +11,18 @@ import (
 )
 
 type input struct {
-	fullFilepath string
-	data         []byte
+	inputFullFilepath  string
+	outputFullFilepath string
+	data               []byte
 }
 
 // E.g. "website.html" -> "website"
 func fileNameWithoutExtension(fileName string) string {
 	return strings.TrimSuffix(fileName, filepath.Ext(fileName))
 }
 
-func getOutputFileName(fullFilepath string) string {
-	basenameWithExt := filepath.Base(fullFilepath)
-	basename := fileNameWithoutExtension(basenameWithExt)
-
-	return basename + ".md"
-}
+// If the output is a file, it would be "output.md"
+var defaultBasename = "output"
 
 func (cli *CLI) listInputs() ([]*input, error) {
 	if cli.isStdinPipe && cli.config.inputFilepath != "" {
@@ -41,9 +38,8 @@ func (cli *CLI) listInputs() ([]*input, error) {
 		}
 		return []*input{
 			{
-				// If the output is a file, it would be "output.md"
-				fullFilepath: "output",
-				data:         data,
+				inputFullFilepath: defaultBasename,
+				data:              data,
 			},
 		}, nil
 	}
@@ -54,6 +50,16 @@ func (cli *CLI) listInputs() ([]*input, error) {
 			return nil, err
 		}
 		if len(matches) == 0 {
+			// The inputFilepath wasn't actually a glob but was pointing to an existing folder.
+			// The user probably wanted to convert all files in that folder — so we recommend the glob.
+			if outInfo, err := os.Stat(cli.config.inputFilepath); err == nil && outInfo.IsDir() {
+				return nil, NewCLIError(
+					fmt.Errorf("input path %q is a directory, not a file", cli.config.inputFilepath),
+					Paragraph("Here is how you can use a glob to match multiple files:"),
+					CodeBlock(`html2markdown --input "src/*.html" --output "dist/"`),
+				)
+			}
+
 			return nil, NewCLIError(
 				fmt.Errorf("no files found matching pattern %q", cli.config.inputFilepath),
 				Paragraph("Here is how you can use a glob to match multiple files:"),
@@ -64,8 +70,8 @@ func (cli *CLI) listInputs() ([]*input, error) {
 		var inputs []*input
 		for _, match := range matches {
 			inputs = append(inputs, &input{
-				fullFilepath: match,
-				data:         nil,
+				inputFullFilepath: match,
+				data:              nil,
 			})
 		}
 
@@ -84,7 +90,7 @@ func (cli *CLI) readInput(in *input) ([]byte, error) {
 		return in.data, nil
 	}
 
-	data, err := os.ReadFile(in.fullFilepath)
+	data, err := os.ReadFile(in.inputFullFilepath)
 	if err != nil {
 		return nil, err
 	}