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

Fix: the guard-lf-intact-major-modes where not taken into account #2

Merged
merged 2 commits into from
Jun 21, 2024

Conversation

abougouffa
Copy link
Contributor

This PR fixes the issue mentioned below:

While trying to add some modes (like pcap-mode) to guard-lf-intact-major-modes, I've noticed that it wasn't taken into account and that the mode was always changed to fundamental-mode.

After a quick check, it turned out that guard-lf-intact-major-modes wasn't used correctly in provided-mode-derived-p, which needs the modes (&rest modes) to be provided as separate arguments and not as a list.

The signature of the function:

(provided-mode-derived-p MODE &rest MODES)

This PR fixes the issue mentioned below:

While trying to add some modes (like `pcap-mode`) to `guard-lf-intact-major-modes`, I've noticed that it wasn't taken into account and that the mode was always changed to `fundamental-mode`.

After a quick check, it turned out that `guard-lf-intact-major-modes` wasn't used correctly in `provided-mode-derived-p`, which needs the modes (`&rest modes`) to be provided as separate arguments and not as a list.

The signature of the function:
``` emacs-lisp
(provided-mode-derived-p MODE &rest MODES)
```
@jcs090218
Copy link
Member

How about?

(defun guard-lf--set-auto-mode-0 (fnc &rest args)
  "Advice around the function `set-auto-mode-0'.

Arguments FNC and ARGS are used to call original operations."
  (when guard-lf--detect-large-file
    (setq guard-lf--detect-large-file nil)  ; Revert back to `nil'
    (when (and guard-lf-major-mode
-               (not (provided-mode-derived-p (nth 0 args) guard-lf-intact-major-modes)))
+               (not (derived-mode-p (nth 0 args) guard-lf-intact-major-modes)))
      (message "[INFO] Large file detected; use the `%s' as the new major mode"
               guard-lf-major-mode)
      (setf (nth 0 args) guard-lf-major-mode)))
  (apply fnc args))

@abougouffa
Copy link
Contributor Author

abougouffa commented Jun 21, 2024

Well, it won't work for two reasons:

  1. derived-mode-p has also a definition similar to provided-derived-mode-p, it requires several arguments, not a list:
(derived-mode-p 'c-mode 'c++-mode 'python-mode) ;; Correct => it returns t if the current's buffer `major-mode` is derived from c-mode, c++-mode or python-mode

(derived-mode-p '(c-mode c++-mode 'python-mode)) ;; Incorrect => returns (almost) always nil
  1. derived-mode-p is just a convenient function around provided-derived-mode-p, it compares the current major-mode to the other modes, so:
(provided-derived-mode-p major-mode 'c-mode 'c++-mode 'python-mode)

;; Is equivalent to:

(derived-mode-p 'c-mode 'c++-mode 'python-mode)

So, even if we want to use derived-mode-p (which we don't want since at set-auto-mode-0, the major-mode isn't detected yet, we need to do it with an (apply #'derived-mode-p (append (car args) guard-lf-intact-major-modes))

The apply function takes a function and a list of arguments, and use them as arguments to to call the function, so:

(apply #'+ '(1 2 3 4 5)) ; => 15

;; Is equivalent to:

(+ 1 2 3 4 5) ; => 15

While, doing:

(setq l1 '(1 2)
      l2 '(2 3 4 5))

(+ (car l1) l2) ; which is equivalent to what we have now in the code

Is not a valid Elisp expression

@abougouffa
Copy link
Contributor Author

You can set up a test like this:

  • Set the limit:
(setq large-file-warning-threshold 10000) ; 10k
  • C-x C-f and open a .c file larger than 10k
    • The file should be opened by guard-lf in fundamental-mode
  • Kill the 10k file buffer
  • Run this:
(add-to-list 'guard-lf-intact-major-modes 'c-mode)
(add-to-list 'guard-lf-intact-major-modes 'c-ts-mode)
  • C-x C-f open the 10k .c file
    • It will be opened in fundamental-mode, the guard-lf-intact-major-modes is not respected

abougouffa added a commit to abougouffa/minemacs that referenced this pull request Jun 21, 2024
@jcs090218
Copy link
Member

Thank you for your explanation. Sorry, I was a little busy and wasn't paying enough attention to this. 😓

@jcs090218 jcs090218 merged commit 2df42fa into jcs-elpa:master Jun 21, 2024
10 of 11 checks passed
@abougouffa
Copy link
Contributor Author

Thanks for your responsiveness @jcs090218 !

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

Successfully merging this pull request may close these issues.

2 participants