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

Replace title and table sregexp to rx samples #31

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## master (unreleased)

## 0.7.1 (2023-04-05)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for this release header, as it will be added on release time. The heading here should actually be "Bugs fixed".


- [#24](https://github.com/bbatsov/adoc-mode/issues/24): Fix regexp for tables font-locking.

### New features

- [#21](https://github.com/bbatsov/adoc-mode/pull/21): Add support for native font-locking in code blocks.
Expand Down
109 changes: 98 additions & 11 deletions adoc-mode.el
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,10 @@

(require 'cl-lib)
(require 'tempo)
(require 'rx)

(defconst adoc-mode-version "0.8.0-snapshot"
"adoc mode version number.
"`adoc-mode' version number.

Based upon AsciiDoc version 8.5.2. I.e. regexeps and rules are
taken from that version's asciidoc.conf / manual.")
Expand Down Expand Up @@ -321,9 +322,9 @@ aligned.
(defvar adoc-align-face 'adoc-align-face)

;; Despite the comment in font-lock.el near 'defvar font-lock-comment-face', it
;; seems I still need variables to refer to faces in adoc-font-lock-keywords.
;; seems I still need variables to refer to faces in adoc-mode-font-lock-keywords.
;; Not having variables and only referring to face names in
;; adoc-font-lock-keywords does not work.
;; adoc-mode-font-lock-keywords does not work.
(defvar adoc-delimiter 'adoc-meta-face)
(defvar adoc-hide-delimiter 'adoc-meta-hide-face)

Expand Down Expand Up @@ -369,8 +370,77 @@ customizable.")
(defvar adoc-mode-abbrev-table nil
"Abbrev table in use in adoc-mode buffers.")

(defvar adoc-font-lock-keywords nil
"Font lock keywords in adoc-mode buffers.")
(defconst adoc-title-0-regexp
(rx bol
"="
space
(one-or-more any)
eol)
"Regular expression for AsciiDoc Document Title (Level 0).")

(defconst adoc-title-1-regexp
(rx bol
"=="
space
(one-or-more any)
eol)
"Regular expression for AsciiDoc Level 1 Section Title.")

(defconst adoc-title-2-regexp
(rx bol
"==="
space
(one-or-more any)
eol)
"Regular expression for AsciiDoc Level 2 Section Title.")

(defconst adoc-title-3-regexp
(rx bol
"===="
space
(one-or-more any)
eol)
"Regular expression for AsciiDoc Level 3 Section Title.")

(defconst adoc-title-4-regexp
(rx bol
"===="
space
(one-or-more any)
eol)
"Regular expression for AsciiDoc Level 4 Section Title.")

(defconst adoc-title-5-regexp
(rx bol
"====="
space
(one-or-more any)
eol)
"Regular expression for AsciiDoc Level 5 Section Title.")

(defconst adoc-table-regexp
(rx (seq
(minimal-match
bol
"|==="
eol
(zero-or-more anything)
bol
"|==="
eol)))
"Regular expression for AsciiDoc table.")

(defconst adoc-code-regexp
Copy link
Collaborator

@TobiasZawada TobiasZawada Aug 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side-note: Without source-style these are actually listings, see https://docs.asciidoctor.org/asciidoc/latest/blocks/delimited/.

(rx (seq
(minimal-match
bol
"----"
Copy link
Collaborator

@TobiasZawada TobiasZawada Aug 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This rigid kind of regexp is wrong.
The fence is actually "----+" and the end fence must exactly match the start fence.
This makes nested blocks possible.
See https://docs.asciidoctor.org/asciidoc/latest/blocks/delimited/#linewise-delimiters.

eol
(zero-or-more anything)
bol
"----"
eol)))
"Regular expression for AsciiDoc code block.")

(defvar adoc-replacement-failed nil )

Expand Down Expand Up @@ -766,13 +836,13 @@ language.
(defvar adoc-verbatim-face 'adoc-verbatim-face)

(defface adoc-warning-face
'((t :inherit (font-lock-warning-face)))
'((t :inherit font-lock-warning-face))
"For things that should stand out"
:group 'adoc-faces)
(defvar adoc-warning-face 'adoc-warning-face)

(defface adoc-table-face
'((t (:inherit (adoc-code-face))))
'((t (:inherit fixed-pitch)))
"Face for tables."
:group 'adoc-faces)
(defvar adoc-table-face 'adoc-table-face)
Expand Down Expand Up @@ -934,6 +1004,7 @@ or for footnotes, or for floating text."
;; from asciidoc.conf: ^= +(?P<title>[\S].*?)( +=)?$
;; asciidoc src code: Title.isnext reads two lines, which are then parsed by
;; Title.parse. The second line is only for the underline of two line titles.

(defun adoc-re-one-line-title (level)
"Returns a regex matching a one line title of the given LEVEL.
When LEVEL is nil, a one line title of any level is matched.
Expand All @@ -947,7 +1018,7 @@ match-data has these sub groups:

== my title == n
---12------23------
4--4"
4--4"
(let* ((del (if level
(make-string (+ level 1) ?=)
(concat "=\\{1," (+ adoc-title-max-level 1) "\\}"))))
Expand Down Expand Up @@ -1002,7 +1073,7 @@ DEL is described in `adoc-re-two-line-title-undlerline'.
match-data has these sub groups:

1 dummy, so that group 2 is the title's text as in
adoc-re-one-line-title
adoc-re-one-line-title
2 title's text
3 delimiter
0 only chars that belong to the title block element"
Expand Down Expand Up @@ -1112,7 +1183,7 @@ Subgroups:
3 delimiter incl trailing whites
4 delimiter only

foo :: bar
foo :: bar
-12--23-3
44"
(cond
Expand Down Expand Up @@ -2676,7 +2747,7 @@ new customization demands."
(null adoc-unichar-alist))
(adoc-make-unichar-alist))

(setq adoc-font-lock-keywords (adoc-get-font-lock-keywords))
(setq adoc-mode-font-lock-keywords (adoc-get-font-lock-keywords))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you can see we intentionally don't use variables named adoc-mode-something most of the time, so we can have shorter identifiers. Please, don't rename this variable.

(when (and font-lock-mode (eq major-mode 'adoc-mode))
(font-lock-flush)
(font-lock-ensure)))
Expand Down Expand Up @@ -3416,6 +3487,22 @@ LOCAL-ATTRIBUTE-FACE-ALIST before it is looked up in
"Keymap used in adoc mode.")


(defcustom adoc-mode-font-lock-keywords
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this have the same name as the variable that you set above? Seems incorrect to me.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable adoc-mode-font-lock-keywords is used with (setq adoc-mode-font-lock-keywords...) but not defined. I don't see code like this:

(defvar adoc-mode-font-lock-keywords ...)

I used example from dns-mode.el for writing this code. Also I saw some TODO's like this:

1539 ;; TODO: use same regexps as for font lock

I think, it is a good idea replace functions for regexp to constants with more human readable (and maintain!) code, based on rx.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you didn't get my point - this defcustom has some value that simple gets overwritten here:

(setq adoc-mode-font-lock-keywords (adoc-get-font-lock-keywords))

I'm fine with simplifying code by using rx, but I just don't understand what's the point of this defcustom right now.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, I add before adoc-calc function declaration like this?

(defvar adoc-font-lock-keywords nil "Keywords for font-lock in `adoc-mode'.")

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd probably remove this adoc-calc completely, as few modes do something like this. If you want to factor in some font-lock customizations you typically just disable/enable the mode and its init code will take care of such updates. In general it's very bad style to have a top-level function call to initialize anything (as adoc-mode currently does).

`(
((adoc-title-0-regexp) 0 ,adoc-title-0-face)
((adoc-title-1-regexp) 0 ,adoc-title-1-face)
((adoc-title-2-regexp) 0 ,adoc-title-2-face)
((adoc-title-3-regexp) 0 ,adoc-title-3-face)
((adoc-title-4-regexp) 0 ,adoc-title-4-face)
((adoc-title-5-regexp) 0 ,adoc-title-5-face)
((adoc-table-regexp) 0 ,adoc-table-face)
((adoc-code-regexp) 0 ,adoc-code-face))
"Font lock keywords used to highlight text in AsciiDoc files mode."
:version "0.8.0-snapshot"
:type 'sexp
:group 'adoc-mode)


;;;###autoload
(define-derived-mode adoc-mode text-mode "adoc"
"Major mode for editing AsciiDoc text files.
Expand Down