plugin(table): add option to remove padding from tables (2) (#184)

Author: JohannesKaufmann

cli/html2markdown/cmd/cmd_convert.go

@@ -112,6 +112,7 @@ func (cli *CLI) convert(input []byte) ([]byte, error) {
 				table.WithSpanCellBehavior(table.SpanCellBehavior(cli.config.tableSpanCellBehavior)),
 				table.WithPresentationTables(cli.config.tablePresentationTables),
 				table.WithNewlineBehavior(table.NewlineBehavior(cli.config.tableNewlineBehavior)),
+				table.WithCellPaddingBehavior(table.CellPaddingBehavior(cli.config.tableCellPaddingBehavior)),
 			),
 		)
 	}

cli/html2markdown/cmd/exec.go

@@ -40,12 +40,13 @@ type Config struct {
 	// - - - - - Plugins - - - - - //
 	enablePluginStrikethrough bool
 
-	enablePluginTable       bool
-	tableSkipEmptyRows      bool
-	tableHeaderPromotion    bool
-	tableSpanCellBehavior   string
-	tablePresentationTables bool
-	tableNewlineBehavior    string
+	enablePluginTable        bool
+	tableSkipEmptyRows       bool
+	tableHeaderPromotion     bool
+	tableSpanCellBehavior    string
+	tablePresentationTables  bool
+	tableNewlineBehavior     string
+	tableCellPaddingBehavior string
 }
 
 // Release holds the information (from the 3 ldflags) that goreleaser sets.

cli/html2markdown/cmd/flags.go

@@ -94,6 +94,7 @@ func (cli *CLI) initFlags(progname string) {
 	cli.flags.StringVar(&cli.config.tableSpanCellBehavior, "opt-table-span-cell-behavior", "", `[for --plugin-table] how colspan/rowspan should be rendered: "empty" or "mirror"`)
 	cli.flags.BoolVar(&cli.config.tablePresentationTables, "opt-table-presentation-tables", false, `[for --plugin-table] whether tables with role="presentation" should be converted`)
 	cli.flags.StringVar(&cli.config.tableNewlineBehavior, "opt-table-newline-behavior", "", `[for --plugin-table] how tables containing newlines should be handled: "skip" or "preserve"`)
+	cli.flags.StringVar(&cli.config.tableCellPaddingBehavior, "opt-table-cell-padding-behavior", "", `[for --plugin-table] whether cells in the tables should include extra padding for visual continuity: "aligned", "minimal", or "none"`)
 }
 
 func (cli *CLI) parseFlags(args []string) error {
@@ -120,6 +121,9 @@ func (cli *CLI) parseFlags(args []string) error {
 	if cli.config.tableNewlineBehavior != "" && !cli.config.enablePluginTable {
 		return fmt.Errorf("--opt-table-newline-behavior requires --plugin-table to be enabled")
 	}
+	if cli.config.tableCellPaddingBehavior != "" && !cli.config.enablePluginTable {
+		return fmt.Errorf("--opt-table-cell-padding-behavior requires --plugin-table to be enabled")
+	}
 
 	// TODO: use constant for flag name & use formatFlag
 	//       var keyStrongDelimiter = "opt-strong-delimiter"

cli/html2markdown/cmd/testdata/TestExecute/[general]_help_pipe/stdout.golden

@@ -72,6 +72,9 @@ Use a HTML sanitizer before displaying the HTML in the browser!
         Make bold text. Should <strong> be indicated by two asterisks or two underscores?
         "**" or "__" (default: "**")
 
+    --opt-table-cell-padding-behavior
+        [for --plugin-table] whether cells in the tables should include extra padding for visual continuity: "aligned", "minimal", or "none"
+
     --opt-table-header-promotion
         [for --plugin-table] first row should be treated as a header
 

cli/html2markdown/cmd/testdata/TestExecute/[general]_help_terminal/stdout.golden

@@ -72,6 +72,9 @@ Use a HTML sanitizer before displaying the HTML in the browser!
         Make bold text. Should <strong> be indicated by two asterisks or two underscores?
         "**" or "__" (default: "**")
 
+    --opt-table-cell-padding-behavior
+        [for --plugin-table] whether cells in the tables should include extra padding for visual continuity: "aligned", "minimal", or "none"
+
     --opt-table-header-promotion
         [for --plugin-table] first row should be treated as a header
 

plugin/table/3_render.go

@@ -73,7 +73,11 @@ func (s *tablePlugin) writeHeaderUnderline(w converter.Writer, alignments []stri
 			w.WriteString("-")
 		}
 
-		w.WriteString(strings.Repeat("-", maxLength))
+		if s.cellPaddingBehavior == CellPaddingBehaviorAligned {
+			w.WriteString(strings.Repeat("-", maxLength))
+		} else {
+			w.WriteString("-")
+		}
 
 		if align == "right" || align == "center" {
 			w.WriteString(":")
@@ -90,16 +94,24 @@ func (s *tablePlugin) writeRow(w converter.Writer, counts []int, cells [][]byte)
 		if isFirstCell {
 			w.WriteString("|")
 		}
-		w.WriteString(" ")
-		w.Write(cell)
 
 		currentCount := utf8.RuneCount(cell)
 		filler := counts[i] - currentCount
 
-		if filler > 0 {
+		if s.cellPaddingBehavior == CellPaddingBehaviorAligned || s.cellPaddingBehavior == CellPaddingBehaviorMinimal {
+			w.WriteString(" ")
+		}
+
+		w.Write(cell)
+
+		if s.cellPaddingBehavior == CellPaddingBehaviorAligned && filler > 0 {
 			w.WriteString(strings.Repeat(" ", filler))
 		}
 
-		w.WriteString(" |")
+		if s.cellPaddingBehavior == CellPaddingBehaviorAligned || s.cellPaddingBehavior == CellPaddingBehaviorMinimal {
+			w.WriteString(" ")
+		}
+
+		w.WriteString("|")
 	}
 }

plugin/table/table.go

@@ -71,6 +71,39 @@ func WithNewlineBehavior(behavior NewlineBehavior) option {
 	}
 }
 
+type CellPaddingBehavior string
+
+const (
+	// CellPaddingBehaviorAligned adds visual padding to cells to make each column equal width (default).
+	CellPaddingBehaviorAligned CellPaddingBehavior = "aligned"
+	// CellPaddingBehaviorMinimal keeps a very small amount of padding to balance table readability while also reducing character count.
+	CellPaddingBehaviorMinimal CellPaddingBehavior = "minimal"
+	// CellPaddingBehaviorNone refrains from adding the padding to the cells.
+	CellPaddingBehaviorNone CellPaddingBehavior = "none"
+)
+
+// WithCellPaddingBehavior configures how to handle padding in table cells.
+// When set to "aligned" (default), every cell's text is padded to the width of the largest cell in its column.
+// When set to "minimal", every cell gets a space at the beginning and end of the cell for some minimal padding.
+// When set to "none", no extra padding is applied to cells.
+func WithCellPaddingBehavior(behavior CellPaddingBehavior) option {
+	return func(p *tablePlugin) error {
+		switch behavior {
+		case "":
+			// Allow empty string to default to "aligned"
+			p.cellPaddingBehavior = CellPaddingBehaviorAligned
+			return nil
+
+		case CellPaddingBehaviorAligned, CellPaddingBehaviorMinimal, CellPaddingBehaviorNone:
+			p.cellPaddingBehavior = behavior
+			return nil
+
+		default:
+			return fmt.Errorf("unknown value %q for cell padding behavior", behavior)
+		}
+	}
+}
+
 // WithSkipEmptyRows configures the table plugin to omit empty rows from the output.
 // An empty row is defined as a row where all cells contain no content or only whitespace.
 // When set to true, empty rows will be omitted from the output. When false (default),
@@ -113,6 +146,7 @@ type tablePlugin struct {
 	skipEmptyRows             bool
 	promoteFirstRowToHeader   bool
 	convertPresentationTables bool
+	cellPaddingBehavior       CellPaddingBehavior
 }
 
 func (p *tablePlugin) setError(err error) {
@@ -129,7 +163,9 @@ func (p *tablePlugin) getError() error {
 }
 
 func NewTablePlugin(opts ...option) converter.Plugin {
-	plugin := &tablePlugin{}
+	plugin := &tablePlugin{
+		cellPaddingBehavior: CellPaddingBehaviorAligned,
+	}
 	for _, opt := range opts {
 		err := opt(plugin)
 		if err != nil {

plugin/table/table_test.go

@@ -620,7 +620,7 @@ B1 B2
 	}
 }
 
-func TestTableWithNewlines(t *testing.T) {
+func TestOptionFunc_NewlineBehavior(t *testing.T) {
 	testCases := []struct {
 		desc     string
 		input    string
@@ -689,3 +689,107 @@ A12
 		})
 	}
 }
+
+func TestOptionFunc_CellPaddingBehavior(t *testing.T) {
+	testCases := []struct {
+		desc     string
+		input    string
+		options  []option
+		expected string
+	}{
+		{
+			desc:    "with padding behavior (default)",
+			options: []option{},
+			input: `
+<table>
+  <tr>
+    <td>This line has some way longer text than the other line below it.</td>
+    <td>A2</td>
+  </tr>
+  <tr>
+    <td>B1</td>
+    <td>This one has longer text than the line above.</td>
+  </tr>
+</table>
+			`,
+			expected: `
+|                                                                  |                                               |
+|------------------------------------------------------------------|-----------------------------------------------|
+| This line has some way longer text than the other line below it. | A2                                            |
+| B1                                                               | This one has longer text than the line above. |
+			`,
+		},
+		{
+			desc: "with minimal padding behavior",
+			options: []option{
+				WithCellPaddingBehavior(CellPaddingBehaviorMinimal),
+			},
+			input: `
+<table>
+  <tr>
+    <td>This line has some way longer text than the other line below it.</td>
+    <td>A2</td>
+  </tr>
+  <tr>
+    <td>B1</td>
+    <td>This one has longer text than the line above.</td>
+  </tr>
+</table>
+			`,
+			expected: `
+|  |  |
+|---|---|
+| This line has some way longer text than the other line below it. | A2 |
+| B1 | This one has longer text than the line above. |
+`,
+		},
+		{
+			desc: "without padding behavior",
+			options: []option{
+				WithCellPaddingBehavior(CellPaddingBehaviorNone),
+			},
+			input: `
+<table>
+  <tr>
+    <td>This line has some way longer text than the other line below it.</td>
+    <td>A2</td>
+  </tr>
+  <tr>
+    <td>B1</td>
+    <td>This one has longer text than the line above.</td>
+  </tr>
+</table>
+			`,
+			expected: `
+|||
+|---|---|
+|This line has some way longer text than the other line below it.|A2|
+|B1|This one has longer text than the line above.|
+`,
+		},
+	}
+
+	for _, tC := range testCases {
+		t.Run(tC.desc, func(t *testing.T) {
+			conv := converter.NewConverter(
+				converter.WithPlugins(
+					base.NewBasePlugin(),
+					commonmark.NewCommonmarkPlugin(),
+					NewTablePlugin(tC.options...),
+				),
+			)
+
+			output, err := conv.ConvertString(tC.input)
+			if err != nil {
+				t.Error(err)
+			}
+
+			actual := strings.TrimSpace(output)
+			expected := strings.TrimSpace(tC.expected)
+
+			if actual != expected {
+				t.Errorf("expected\n%s\nbut got\n%s\n", expected, actual)
+			}
+		})
+	}
+}