bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#60983: 29.0.60; Tree-sitter user-level control


From: Stefan Monnier
Subject: bug#60983: 29.0.60; Tree-sitter user-level control
Date: Wed, 25 Jan 2023 16:16:33 -0500
User-agent: Gnus/5.13 (Gnus v5.13)

> -(defcustom c-ts-mode-indent-style 'gnu
> +(defcustom c-ts-mode-indent-style "gnu"
>    "Style used for indentation.
>  
>  The selected style could be one of GNU, K&R, LINUX or BSD.  If
> @@ -100,13 +100,33 @@ c-ts-mode-indent-style
>  set instead.  This function is expected return a list that
>  follows the form of `treesit-simple-indent-rules'."
>    :version "29.1"
> -  :type '(choice (symbol :tag "Gnu" 'gnu)
> -                 (symbol :tag "K&R" 'k&r)
> -                 (symbol :tag "Linux" 'linux)
> -                 (symbol :tag "BSD" 'bsd)
> +  :type '(choice (string :tag "Gnu" "gnu")
> +                 (string :tag "K&R" "k&r")
> +                 (string :tag "Linux" "linux")
> +                 (string :tag "BSD" "bsd")
>                   (function :tag "A function for user customized style" 
> ignore))
> +  :set #'c-ts-mode--indent-style
>    :group 'c)

Why change to strings?
BTW the previous code seems wrong: instead of

    (symbol :tag "FOO" 'foo)

it should be

    (symbol :tag "FOO" foo)

since `'foo` is not a symbol but a list (of two symbols).

> +(defun c-ts-mode--indent-style (sym val)
> +  "Custom setter for `c-ts-mode-indent-style'."
> +  (set-default sym val))

Hmm... why bother use a `:set`ter if it doesn't do anything more than
the default does?
Shouldn't it call `treesit--indent-rules-optimize` to (re)set
`treesit-simple-indent-rules`?
[ Presumably in all relevant buffers, since the defcustom setting is
  global.  ]

> +(defun c-ts-mode-set-style ()
> +  (interactive)
> +  (or (eq major-mode 'c-ts-mode) (eq major-mode 'c++-ts-mode)
> +      (error "Buffer %s is not a c-ts-mode (c-ts-mode-set-style)"
> +             (buffer-name)))
> +  (if-let ((mode (cond ((eq major-mode 'c-ts-mode) 'c)
> +                       ((eq major-mode 'c++-ts-mode) 'cpp)
> +                       (t nil)))
> +           (choice (completing-read "Select style: " '("gnu" "k&r" "linux" 
> "bsd"))))

Here, we probably want to specify `must-match` to `completing-read`
(which makes it unnecessary to check `if-let`, I think) and we should
provide a default.
Also we should probably use the (c-ts-mode--indent-styles mode) alist
rather than hardcode the set of styles.

> +      (c-ts-mode--indent-style 'c-ts-mode-indent-style choice)
> +    (kill-local-variable 'treesit-simple-indent-rules)
> +    (setq-local treesit-simple-indent-rules
> +                (treesit--indent-rules-optimize
> +                 (c-ts-mode--set-indent-style mode)))))

Here we presumably want to do the (setq-local
treesit-simple-indent-rules ...) every time (and set
`c-ts-mode-indent-style` buffer locally rather than via
`c-ts-mode--indent-style`, or otherwise provide an additional arg to
`c-ts-mode--indent-style` to say whether it applies globally or only to
the current buffer).

BTW, the naming of `c-ts-mode--indent-style` and
`c-ts-mode--set-indent-style` is confusing.

Also: why `kill-local-variable` just before the `setq-local`?

> -           (pcase c-ts-mode-indent-style
> -             ('gnu   (alist-get 'gnu (c-ts-mode--indent-styles mode)))
> -             ('k&r   (alist-get 'k&r (c-ts-mode--indent-styles mode)))
> -             ('bsd   (alist-get 'bsd (c-ts-mode--indent-styles mode)))
> -             ('linux (alist-get 'linux (c-ts-mode--indent-styles mode)))))))
> +           (alist-get c-ts-mode-indent-style
> +                      (c-ts-mode--indent-styles mode) nil nil #'equal))))

Thanks :-)


        Stefan






reply via email to

[Prev in Thread] Current Thread [Next in Thread]