melpa versoin of this mode does not associate itself to *.pp files automatically #10

Open
wants to merge 1 commit into
from

Projects

None yet

8 participants

@shishirsharma

#9

melpa version of this mode does not associate itself to *.pp files automatically.
melpa builds its files directly from this. So this mode should be updated

@purcell

+1 on this!

@nicferrier

There's a discussion on reddit currently about automatic configuration, people are in two minds about it.

Personally, I think you should do what you want like this:


(defcustom puppet-mode-add-to-auto-mode-alist "\\.pp$"
    "Make `puppet-mode' associated with pp files."
    :type '(radio
            (const :tag "Off" nil)
            (regexp "\\.pp$")))

(when puppet-mode-add-to-auto-mode-alist
  (add-to-list
   'auto-mode-alist
   (cons puppet-mode-add-to-auto-mode-alist 'puppet-mode)))
@purcell

@nicferrier I respectfully disagree. That's inconsistent with how both other third-party libs and those bundled with Emacs modify auto-mode-alist.

But more importantly, as far as I can see it won't work if a variable customisation is applied after the library gets loaded, e.g.

(require 'puppet-mode)
(setq puppet-mode-add-to-auto-mode-alist nil) ;; or (load custom-file)
;; => auto-mode-alist has still been modified

Customisation should have the same effect whether it's applied before or after a library has been loaded.

@nicferrier

Right. But it's also easy to add :set and :get functions. In fact, that's maybe even better:

(defcustom puppet-mode-add-to-auto-mode-alist "\\.pp$"
  "Make `puppet-mode' associated with pp files."
  :group 'puppet
  :type '(radio
          (const :tag "Off" nil)
          (regexp "\\.pp$"))
  :set (lambda (sym regex)
         (if (not regex)
             (setq auto-mode-alist
                   (rassq-delete-all
                    'puppet-mode auto-mode-alist))
             ;; else
             (setq auto-mode-alist
                   (rassq-delete-all
                    'puppet-mode auto-mode-alist))
             (add-to-list
              'auto-mode-alist
              (cons regex 'puppet-mode)))))
@milkypostman

can't you change an auto-mode-alist entry by just adding a new one?

@nicferrier

You can, but if you were using it with customize and flicking it on and off it might get big. That code keeps it clean. It's not that hard.

@leelynne

+1. I was surprised when it wasn't automatically associated after installing it from melpa.

@puppetcla

Waiting for CLA signature by @expertmind

@expertmind - We require a Contributor License Agreement (CLA) for people who contribute to Puppet, but we have an easy click-through license with instructions, which is available at https://cla.puppetlabs.com/

Note: if your contribution is trivial and you think it may be exempt from the CLA, please post a short reply to this comment with details. http://docs.puppetlabs.com/community/trivial_patch_exemption.html

@shishirsharma
@puppetcla

CLA signed by all contributors.

@lunaryorn lunaryorn referenced this pull request in lunaryorn/puppet-mode Aug 18, 2013
Closed

Review open tickets and pull request in the original repo #4

@shishirsharma

Does this means this Repo is now deprecated?

@bbatsov

Yes.

@mcandre

+1

@purcell

FWIW, MELPA builds packages from @lunaryorn's puppet-mode repo these days.

@mcandre

It all works now! Turns out my error was using ~/.emacs.d$ cask install instead of M-: (cask-install).

@purcell

@mcandre Aha. Was that "user error", or a cask issue? If the latter, did you mention it to @rejeep?

@mcandre

User error :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment