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.

Sign up for GitHub

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

Indentation and completion improvements #15

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
+360 −71
Open

Indentation and completion improvements #15

Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
ed0099e
Fix indentation of directives not preceeded by empty line
alexsmith1612 May 10, 2021
7017a94
Indent posting metadata lines an additional level
alexsmith1612 May 10, 2021
fdfa9c2
Align numbers in balance and price directives in addition to postings
alexsmith1612 May 10, 2021
b4f14e4
Align open directive currencies to number alignment + 1
alexsmith1612 May 10, 2021
80a3e8b
Add customizable variable for minimum number alignment separation
alexsmith1612 May 10, 2021
01c5a74
Add customizable variable to enable number alignment with tab-dwim
alexsmith1612 May 10, 2021
013e57a
Add more generic beancount-indent-directive function
alexsmith1612 May 10, 2021
90c54d4
Always call indent-region on TAB if the region is active
alexsmith1612 May 10, 2021
aff0255
Update indentation tests
alexsmith1612 May 10, 2021
c232302
Fix link-at-point-003 unit test
alexsmith1612 May 10, 2021
ee36fcc
Explicitly require imenu in beancount.el
alexsmith1612 May 10, 2021
2489f46
Require org for org-level-x faces
alexsmith1612 May 10, 2021
6df3934
Don't insert space after completions
alexsmith1612 May 10, 2021
7d07c48
Limit non-timestamped directives completions
alexsmith1612 May 10, 2021
2518f70
Allow completion of tags and links not on their own line
alexsmith1612 May 10, 2021
c9725e3
Move posting completions after tag and link completions
alexsmith1612 May 10, 2021
c880cf6
Don't attempt completion inside strings except for options
alexsmith1612 May 10, 2021
4102a12
Don't collect matches inside strings
alexsmith1612 May 10, 2021
a721f15
Add more completion tests
alexsmith1612 May 10, 2021
cc8ea35
Update copyright and author information
alexsmith1612 May 10, 2021
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
58 changes: 44 additions & 14 deletions beancount.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,16 @@ from the open directive for the relevant account."

(defconst beancount-tag-chars "[:alnum:]-_/.")

(defvar beancount-tag-char-table (make-char-table 'tag-table)
"char-table with non-nil values only for characters allowed in tags")
(set-char-table-range beancount-tag-char-table (cons ?a ?z) t)
(set-char-table-range beancount-tag-char-table (cons ?A ?Z) t)
(set-char-table-range beancount-tag-char-table (cons ?0 ?9) t)
(set-char-table-range beancount-tag-char-table ?- t)
(set-char-table-range beancount-tag-char-table ?_ t)
(set-char-table-range beancount-tag-char-table ?/ t)
(set-char-table-range beancount-tag-char-table ?. t)

(defconst beancount-account-chars "[:alnum:]-_:")

(defconst beancount-option-names
Expand Down Expand Up @@ -448,6 +458,27 @@ With an argument move to the next non cleared transaction."
"A list of the accounts available in this buffer.")
(make-variable-buffer-local 'beancount-accounts)

(defun beancount-inside-string-p ()
(nth 3 (syntax-ppss)))

(defun beancount-looking-at-tag-generic (start-char start-char-string)
(save-excursion
(unless (beancount-inside-string-p)
;; Loop backward until the previous character is not a valid tag
;; character
(while (char-table-range beancount-tag-char-table (char-before))
(backward-char))
;; Move before the start character if we hit it
(when (eq (char-before) start-char)
(backward-char))
(looking-at (concat start-char-string "[" beancount-tag-chars "]*")))))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Cannot this based on (re-search-backward (concat "[^" beancount-tag-chars "]")) instead?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, that is a better solution.

I was so caught up in a "don't use looking-backward" mindset that I seemed to have mentally excluded any sort of backwards regexp search and way overthought the solution.


(defun beancount-looking-at-tag ()
(beancount-looking-at-tag-generic ?# "#"))

(defun beancount-looking-at-link ()
(beancount-looking-at-tag-generic ?^ "\\^"))
Copy link
Collaborator

@dnicolodi dnicolodi May 13, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This functions take the same argument twice. You can use string-to-char or char-to-string instead.

However, I would write this like

(defun bencount--looking-at-tag ()
  (save-excursion
    (re-search-backward (concat "[^" beancount-tag-chars "]"))
    (looking-at (concat "#[" beancount-tag-chars "]"))))

at which point having a generic function is the same lines of code than repeating it. It is probably also wise to put this kind of functions in what is conventionally the private namespace (indicated with the doubly dashed name). This has not been done consistently before, and needs to be fixed.

Copy link
Author

Choose a reason for hiding this comment

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

This functions take the same argument twice. You can use string-to-char or char-to-string instead.

This is not quite true, as in beancount-looking-at-link the string version of the character must be escaped so that it's not treated as a regexp anchor. I initially used char-to-string before realizing that it did not work for links.

However, I would write this like ... at which point having a generic function is the same lines of code than repeating it.

Agreed. Will change.

It is probably also wise to put this kind of functions in what is conventionally the private namespace (indicated with the doubly dashed name). This has not been done consistently before, and needs to be fixed.

I had considered this but wasn't sure whether to make just the private function in my changeset with the double dash namespace or if I should match the convention in (most of) the existing code, potentially updating the full file at once in a separate PR.


(defun beancount-completion-at-point ()
"Return the completion data relevant for the text at point."
(save-excursion
Expand Down Expand Up @@ -499,30 +530,32 @@ With an argument move to the next non cleared transaction."
(list (match-beginning 1) (match-end 1) #'beancount-account-completion-table))

;; tags
((beancount-looking-at
(concat "[ \t]+#\\([" beancount-tag-chars "]*\\)") 1 pos)
((save-excursion
(goto-char pos)
(beancount-looking-at-tag))
(let* ((candidates nil)
(regexp (concat "\\#\\([" beancount-tag-chars "]+\\)"))
(regexp (concat "#[" beancount-tag-chars "]+"))
(completion-table
(lambda (string pred action)
(if (null candidates)
(setq candidates
(sort (beancount-collect regexp 1) #'string<)))
(sort (beancount-collect regexp 0) #'string<)))
(complete-with-action action candidates string pred))))
(list (match-beginning 1) (match-end 1) completion-table)))
(list (match-beginning 0) (match-end 0) completion-table)))

;; links
((beancount-looking-at
(concat "[ \t]+\\^\\([" beancount-tag-chars "]*\\)") 1 pos)
((save-excursion
(goto-char pos)
(beancount-looking-at-link))
(let* ((candidates nil)
(regexp (concat "\\^\\([" beancount-tag-chars "]+\\)"))
(regexp (concat "\\^[" beancount-tag-chars "]+"))
(completion-table
(lambda (string pred action)
(if (null candidates)
(setq candidates
(sort (beancount-collect regexp 1) #'string<)))
(sort (beancount-collect regexp 0) #'string<)))
(complete-with-action action candidates string pred))))
(list (match-beginning 1) (match-end 1) completion-table))))))))
(list (match-beginning 0) (match-end 0) completion-table))))))))

(defun beancount-collect (regexp n)
"Return an unique list of REGEXP group N in the current buffer."
Expand Down Expand Up @@ -990,10 +1023,7 @@ Only useful if you have not installed Beancount properly in your PATH.")
(number-to-string (line-number-at-pos)))))

(defun beancount--bounds-of-link-at-point ()
;; There is no length limit for links but it seems reasonable to
;; limit the search for the link to the 128 characters before and
;; after the point. This number is chosen arbitrarily.
(when (thing-at-point-looking-at (concat "\\^[" beancount-tag-chars "]+") 128)
(when (beancount-looking-at-link)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yup! I just realized how dumb this function is.

(cons (match-beginning 0) (match-end 0))))

(put 'beancount-link 'bounds-of-thing-at-point #'beancount--bounds-of-link-at-point)
Expand Down