Derived mode parent modes are added even if .yas-parents is used

thomasf commented 9 years ago

Use case:

json-mode extends js-mode, I do not want js-snippets in json-mode.

stardiviner commented 9 years ago

I want this feature too.

joaotavora commented 9 years ago

That is intrinsically a bug in json-mode. A good principle about inheritance is to make sure it models the "is a" relationship. From what you're telling me, a JSON file is not really a JS file, it shares some, but not all of its characteristics. So json-mode should be changed to inherit from a some ancestor to js-mode.

That said, sorry for the lecturing and I understand this might be annoying. I will keep this feature request here. @thomasf and @stardiviner should come up with ideas as to how this feature should/could be controlled by the user.

In the meantime, there is a workaround: Have you tried:

creating a really-just-js-mode snippet sub-directory
putting the existing js-mode-only snippets there
adding really-just-js-mode do the .yas-parents of js-mode
Keeping in the js-mode subdir the snippets that apply to both js-mode and json-mode

?

stardiviner commented 9 years ago

This workaround is ok for me.

thomasf commented 9 years ago

maybe an inverse of yas-extra-modes could be added to also disable modes in a local hook?

btw, for this specific instance I think I'm gonna dig up json mode and patch it so that it does not report being inherited from js mode.

joaotavora commented 9 years ago

btw, for this specific instance I think I'm gonna dig up json mode and patch it so that it does not report >being inherited from js mode.

sure, that might bring more problems? Unless you maintain it of course... Oh nevermind I've seen you've opened an issue with it.

Your "inverse" idea is usable, but I'd rather implement a bigger fun while we're at it and provide a bigger gun, like a function or something. yas-extra-modes is deprecated btw. You should use yas-activate-extra-mode and yas-deactivate-extra-mode

joshwnj commented 9 years ago

thanks for the advice @capitaomorte, I'd love to update json-mode so that it handles this case more naturally. Do you know of any examples of other derived modes that inherit correctly, so I can take a look how they do it?

joaotavora commented 9 years ago

Hi @joshwnj, I don't think you're "inheriting incorrectly", it's just that inheritance from js-mode is not quite the correct thing to do in this situation.

You some options:

Make json-mode inherit from noone, then cherry-pick the features of js-mode that you'd think should apply to json-mode.

For example the syntax-table is probably good, but are comments allowed in JSON file? According to this, they aren't. So don't cherry-pick that particular part. Then think about the keymap you want etc etc.
Unfortunately, js-mode's definition inthe current emacs-trunk, for instance, is already rather big, so the previous options will have you repeating a lot of code, which is kind of a nono. The alternative is to ask the author of js-mode and lobby in the emacs-devel mailing list that characteristics setup in js-mode are grouped and packed into logical blocks that can be more easily cherry-picked.
A third alternative very similar to the last one would be to ask the authors to create a common parent kinda like cc-mode is to c-mode, c++-mode and java-mode. It would have the desired common traits. Then make json-mode inherit from there.

While you're lobbying in emacs-devel, why not try for json-mode to be included in Emacs or GNU Elpa? It's a better guarantee of consistency between the two modes.

This is the current js-mode in emacs trunk:

(define-derived-mode js-mode prog-mode "Javascript"
  "Major mode for editing JavaScript."
  :group 'js
  (setq-local indent-line-function 'js-indent-line)
  (setq-local beginning-of-defun-function 'js-beginning-of-defun)
  (setq-local end-of-defun-function 'js-end-of-defun)
  (setq-local open-paren-in-column-0-is-defun-start nil)
  (setq-local font-lock-defaults (list js--font-lock-keywords))
  (setq-local syntax-propertize-function #'js-syntax-propertize)

  (setq-local parse-sexp-ignore-comments t)
  (setq-local parse-sexp-lookup-properties t)
  (setq-local which-func-imenu-joiner-function #'js--which-func-joiner)

  ;; Comments
  (setq-local comment-start "// ")
  (setq-local comment-end "")
  (setq-local fill-paragraph-function 'js-c-fill-paragraph)

  ;; Parse cache
  (add-hook 'before-change-functions #'js--flush-caches t t)

  ;; Frameworks
  (js--update-quick-match-re)

  ;; Imenu
  (setq imenu-case-fold-search nil)
  (setq imenu-create-index-function #'js--imenu-create-index)

  ;; for filling, pretend we're cc-mode
  (setq c-comment-prefix-regexp "//+\\|\\**"
        c-paragraph-start "$"
        c-paragraph-separate "$"
        c-block-comment-prefix "* "
        c-line-comment-starter "//"
        c-comment-start-regexp "/[*/]\\|\\s!"
        comment-start-skip "\\(//+\\|/\\*+\\)\\s *")

  (setq-local electric-indent-chars
          (append "{}():;," electric-indent-chars)) ;FIXME: js2-mode adds "[]*".
  (setq-local electric-layout-rules
          '((?\; . after) (?\{ . after) (?\} . before)))

  (let ((c-buffer-is-cc-mode t))
    ;; FIXME: These are normally set by `c-basic-common-init'.  Should
    ;; we call it instead?  (Bug#6071)
    (make-local-variable 'paragraph-start)
    (make-local-variable 'paragraph-separate)
    (make-local-variable 'paragraph-ignore-fill-prefix)
    (make-local-variable 'adaptive-fill-mode)
    (make-local-variable 'adaptive-fill-regexp)
    (c-setup-paragraph-variables))

  (setq-local syntax-begin-function #'js--syntax-begin-function)

  ;; Important to fontify the whole buffer syntactically! If we don't,
  ;; then we might have regular expression literals that aren't marked
  ;; as strings, which will screw up parse-partial-sexp, scan-lists,
  ;; etc. and produce maddening "unbalanced parenthesis" errors.
  ;; When we attempt to find the error and scroll to the portion of
  ;; the buffer containing the problem, JIT-lock will apply the
  ;; correct syntax to the regular expression literal and the problem
  ;; will mysteriously disappear.
  ;; FIXME: We should actually do this fontification lazily by adding
  ;; calls to syntax-propertize wherever it's really needed.
  (syntax-propertize (point-max)))

joshwnj commented 9 years ago

Thanks @capitaomorte, that's given me a lot to think about

ghost commented 6 years ago

First of all, huge thanks to everyone who has created yasnippet. What an incredible package. You guys rock!

I don't expect this ticket to get solved... but I'd just like to add that the very popular php-mode depends on (require 'cc-mode). I used to like to run with company-yasnippet autocompletion. But that means everything in my menus was polluted. I would type $this-> in PHP and saw stuff like #include <...h> C++ garbage, as if that was part of my class member variables.

I had to turn off company-yasnippet autocompletion to get rid of that, since there was no way for me to turn off the default cc-mode snippets.

Perhaps it's worth thinking about this ticket again... Perhaps some kind of YAS configuration variable for the user to be able to say stuff like "If major mode is php-mode, only load snippets for php-mode (not for other modes in the buffer)".

I think variables make the most sense for this. Because some kind of .yas-parents file on disk is not good enough since YAS can load those from multiple folders (its default folder, the user folders, etc), so it would be hard to configure such a thing inside of a specific folder and figuring out a sane hierarchy. But perhaps doing it via a native Emacs variable would be a good-enough solution. Clearly lots of modes require/derive from other modes, and therefore get nonsense snippets included... ;-\ This problem is saddening.

npostavs commented 6 years ago

I would type $this-> in PHP and saw stuff like #include <...h> C++ garbage, as if that was part of my class member variables.

Maybe most of the cc-mode snippets should be moved to a c-lang-mode directory, which both c-mode and c++-mode would use via .yas-parents, and then none of the other cc-mode derived modes would get them.

ghost commented 6 years ago

@npostavs that would definitely help. All of the c-specific stuff like #include and int main and specific formatted switch and case etc which aren’t compatible with all c-like languages could go in there.

npostavs commented 6 years ago

See AndreaCrotti/yasnippet-snippets#231

ghost commented 6 years ago

@npostavs Thanks for the great work in that pull request. It definitely clears up the problem quite a lot.

In the future I hope we'll have a configure-variable as mentioned here: https://github.com/joaotavora/yasnippet/issues/580#issuecomment-338540349

I'd definitely run with some kinda yasnippet-no-parent-modes '("php-mode") which tells YASnippet to never load parent-mode snippets if php-mode is active in a buffer. That way, people could kill all inheritance in certain modes. It'd be the true permanent solution.

We could then setup a php-mode folder somewhere and just have some neat snippets there. With zero worries about inheriting weird defaults from other modes.

And sure, this kills manual inheritance too, ie if the user creates a snippets folder with php-mode/.yas-parents... But I guess an even better improvement then would be some kind of yasnippet-no-default-snippets '("php-mode") that way it would not load anything from the default core. Maybe that's the best solution.

Either of these would be awesome.

Ping: @AndreaCrotti

joaotavora / yasnippet

Derived mode parent modes are added even if .yas-parents is used #580