Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[markdown] orgtbl-mode incorrectly formats tables on TAB #15256

Closed
practicalli-johnny opened this issue Jan 9, 2022 · 2 comments · Fixed by #15257
Closed

[markdown] orgtbl-mode incorrectly formats tables on TAB #15256

practicalli-johnny opened this issue Jan 9, 2022 · 2 comments · Fixed by #15257

Comments

@practicalli-johnny
Copy link
Contributor

Description :octocat:

Markdown tables use incorrect separation sytax on columns due to orgtbl-mode. This causes the table to not render correctly for static site generators or when viewing the markdown file on the GitHub web page.

orgtbl-mode is automatically enabled with markdown files if the org layer is enabled, forcing markdown TAB key to be hijacked by orgtbl and changing the pipe separator on columns to be a plus character +

The markdown style should be to use the Pipe character as a separator, according to the following Markdown syntax guides

In the layers/+lang/markdown/package.el file, orgtbl-mode is added via a function that uses add-hook to enble orgtbl-mode in markdown mode if the org layer is enabled

(defun markdown/post-init-org ()
  (when (configuration-layer/layer-used-p 'org)
    (add-hook 'markdown-mode-hook 'orgtbl-mode)
    (spacemacs|diminish orgtbl-mode)))

SPC SPC orgtbl-mode in a markdown buffer disables that mode and the TAB key returns to expected operation (the pipe is not changed to a plus character)

In Spacemacs issue #14774 a comment was made by a Spacemacs maintainer stating the orgtbl workaround was now obsolete

Should this `markdown/post-init-org hook is removed from the markdown layer?

Or at least a layer variable be added so that Spacemacs users can configure if orgtbl-mode is available by default.

For example, a variable could be defined to determine if orgtbl-mode should be enabled

(defvar markdown-orgtbl-hijack nil
  "Possibe values are `nil' (markdown table key bindings) or `t' (orgtbl key bindings).")

Add a conditional check in the post-init-org function for the value of this var

(defun markdown/post-init-org ()
  (when (and (configuration-layer/layer-used-p 'org)
             (eq t markdown-orgtbl-hijack))
    (add-hook 'markdown-mode-hook 'orgtbl-mode)
    (spacemacs|diminish orgtbl-mode)))

And add a toggle in the packages list

(defconst markdown-packages
  '(
    company
    company-emoji
    emoji-cheat-sheet-plus
    gh-md
    markdown-mode
    markdown-toc
    mmm-mode
    (org :toggle (eq t markdown-orgtbl-hijack))
    smartparens
    valign
    (vmd-mode :toggle (eq 'vmd markdown-live-preview-engine))))

Please suggest changes or improvements to this code if applicable (this is my first attempt at such a change)

Reproduction guide 🪲

  • Start Emacs
  • Add org and markdown layers
  • Restart Emacs
  • Open a markdown file, .md
  • Create a table and press TAB in one of the cells

Observed behaviour: 👀 💔
TAB key forwards the cursor to the next column and changes the column header separator from a pipe character to a plus character

With orgtbl-mode automatically enabled by the markdown layer, the following table

Syntax Description
Header Title
Paragraph Text

will be formatted with a plus character in the header section column divider

| Syntax | Description |
| ----------- + ----------- |
| Header | Title |
| Paragraph | Text |

This causes the table to not render correctly for static site generators or when viewing the markdown file on the GitHub web page.

Expected behaviour: ❤️ 😄
TAB should forward the cursor to the next column and formats the table layout if required, keeping the header separators as pipe characters

The table should still have the original syntax after pressing TAB

Syntax Description
Header Title
Paragraph Text

System Info 💻

  • OS: gnu/linux
  • Emacs: 27.1
  • Spacemacs: 0.300.0
  • Spacemacs branch: develop (rev. 4d59149)
  • Graphic display: t
  • Distribution: spacemacs
  • Editing style: vim
  • Completion: helm
  • Layers:
(asciidoc
 (auto-completion :variables auto-completion-enable-help-tooltip t auto-completion-enable-snippets-in-popup t auto-completion-enable-sort-by-usage t)
 (clojure :variables cider-repl-display-help-banner nil cider-pprint-fn 'fipp clojure-indent-style 'align-arguments clojure-align-forms-automatically t clojure-toplevel-inside-comment-form t cider-overlays-use-font-lock t cider-repl-buffer-size-limit 100)
 colors command-log csv emacs-lisp emoji
 (git :variables git-magit-status-fullscreen t magit-diff-refine-hunk t git-enable-magit-todos-plugin t)
 github graphviz
 (helm :variables helm-follow-mode-persistent t)
 html javascript
 (json :variables js-indent-level 2 json-backend 'lsp)
 (lsp :variables lsp-enable-on-type-formatting t lsp-enable-indentation t lsp-enable-snippet t lsp-enable-symbol-highlighting t lsp-ui-doc-enable t lsp-ui-doc-show-with-cursor nil lsp-ui-doc-show-with-mouse nil lsp-ui-doc-delay 1 lsp-ui-doc-include-signature t lsp-ui-sideline-enable nil lsp-ui-sideline-show-code-actions nil lsp-lens-enable t treemacs-space-between-root-nodes nil lsp-file-watch-threshold 10000 lsp-log-io nil)
 (markdown :variables markdown-live-preview-engine 'vmd)
 multiple-cursors
 (org :variables org-enable-github-support t org-enable-bootstrap-support t org-enable-reveal-js-support t org-want-todo-bindings t org-enable-org-journal-support t org-journal-dir "~/projects/journal/" org-journal-file-format "%Y-%m-%d" org-journal-date-prefix "#+TITLE: " org-journal-date-format "%A, %B %d %Y" org-journal-time-prefix "* " org-journal-time-format "" org-journal-carryover-items "TODO=\"TODO\"|TODO=\"DOING\"|TODO=\"BLOCKED\"|TODO=\"REVIEW\"")
 (ranger :variables ranger-show-preview t ranger-show-hidden t ranger-cleanup-eagerly t ranger-cleanup-on-disable t ranger-ignored-extensions
         '("mkv" "flv" "iso" "mp4"))
 (shell :variables shell-default-shell 'eshell shell-default-height 30 shell-default-position 'bottom)
 (spacemacs-layouts :variables spacemacs-layouts-restrict-spc-tab t persp-autokill-buffer-on-remove 'kill-weak)
 (spacemacs-modeline :variables doom-modeline-height 12 doom-modeline-major-mode-color-icon t doom-modeline-buffer-file-name-style 'relative-to-project doom-modeline-display-default-persp-name t doom-modeline-minor-modes nil doom-modeline-modal-icon nil)
 spell-checking
 (syntax-checking :variables syntax-checking-use-original-bitmaps t)
 (treemacs :variables treemacs-indentation 1 treemacs-use-git-mode 'simple)
 theming
 (unicode-fonts :variables unicode-fonts-enable-ligatures t unicode-fonts-ligature-modes
                '(prog-mode))
 (version-control :variables version-control-diff-tool 'diff-hl version-control-global-margin t)
 yaml)
  • System configuration features: XPM JPEG TIFF GIF PNG RSVG CAIRO SOUND GPM DBUS GSETTINGS GLIB NOTIFY INOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES THREADS LIBSYSTEMD JSON PDUMPER LCMS2 GMP

Backtrace 🐾

<<BACKTRACE IF RELEVANT>>
@lebensterben
Copy link
Collaborator

lebensterben commented Jan 9, 2022

thanks for your very detailed bug report.

I propose a simple brute-force resolution that we should disable orgtbl entirely for markdown.

Though the intent of its inclusion is to provide some support for markdown table, we should not introduce such feature at the cost of introducing said bug.

And the reason for keeping orgtbl was to avoid existing configs. But apparently avoiding bugs that actually presents now should take priority over preventing potential bugs.

So let's remove it!

@practicalli-johnny
Copy link
Contributor Author

Thank you everyone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants