Puppet: System Administration Automated

Support

Ticket #1185 (closed defect: fixed)

Opened 8 months ago

Last modified 6 months ago

puppet-mode.el issue with array syntax

Reported by: heydrick Assigned to: luke
Priority: normal Milestone: 0.24.5
Component: ext Version: 0.24.4
Severity: normal Keywords: emacs mode puppet-mode.el
Cc: Triage Stage: Ready for checkin
Attached Patches: None Complexity: Easy

Description

puppet-mode.el has problems with array syntax. When I enter an array I get this error in the mini-buffer: "Wrong type argument: number-or-marker-p, "1 occurrences". How to reproduce:

  1. install emacs-mode.el from 0.24.x branch
  2. open foo.pp in emacs
  3. enter in:
class foo { 
  foo => [ "foo", "bar" ],
}

This is with at least emacs 21.4.1 included with Ubuntu Feisty.

Change History

04/10/08 03:30:45 changed by rra

  • owner changed from community to rra.
  • stage changed from Unreviewed to Needs more info.

I can't duplicate this with Emacs 22. I've both pasted in that code and typed it in and don't get any error messages. However, I don't have Emacs 21 installed. Maybe that's the problem?

Could you do M-x set-variable debug-on-error, reproduce this error, and then include the Emacs backtrace in this bug?

04/24/08 04:51:34 changed by heydrick

It must be emacs 21 specific as it works fine for me with 22. Here's the backtrace from emacs 21.4.2.

Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p "1 occurrences")
  =("1 occurrences" 0)
  (if (= (count-matches "\\]" apoint opoint) 0) apoint)
  (progn (if (= ... 0) apoint))
  (if apoint (progn (if ... apoint)))
  (when apoint (if (= ... 0) apoint))
  (let ((opoint ...) (apoint ...)) (when apoint (if ... apoint)))
  (progn (let (... ...) (when apoint ...)))
  (unwind-protect (progn (let ... ...)) (set-match-data save-match-data-internal))
  (let ((save-match-data-internal ...)) (unwind-protect (progn ...) (set-match-data save-match-data-internal)))
  (save-match-data (let (... ...) (when apoint ...)))
  (save-excursion (save-match-data (let ... ...)))
  puppet-in-array()
  (let ((not-indented t) (array-start ...) cur-indent) (cond (array-start ...) (... ... ...) (t ...)) (if cur-inden$
  (if (bobp) (indent-line-to 0) (let (... ... cur-indent) (cond ... ... ...) (if cur-indent ... ...)))
  puppet-indent-line()
  indent-for-tab-command(nil)
* call-interactively(indent-for-tab-command)

04/24/08 05:17:41 changed by rra

  • status changed from new to assigned.
  • complexity changed from Unknown to Easy.

Ah, the problem is that count-matches doesn't do the same thing in emacs21 as it does in emacs22. Hm. That means I need to find a good replacement.

Replacing the call to count-matches with puppet-count-matches and adding:

(defun puppet-count-matches (re start end)
  "The same as Emacs 22 count-matches, for portability to other versions
of Emacs."
  (save-excursion
    (let ((n 0))
      (goto-char start)
      (while (re-search-forward re end t) (incf n))
      n)))

should fix it. Can you give that a try?

04/24/08 07:26:00 changed by luke

  • component changed from documentation to ext.

04/29/08 19:36:06 changed by micah

That does seem to make the "Wrong type argument: number-or-marker-p" error go away, however it no longer handles closing braces properly.

If you open the following in foo.pp:

class foo {

  package {
    "foo":
      ensure => installed;
    }
  }

And you attempt to tab format, you will see that the closing package curly brace is placed in the wrong spot, and the closing class curly brace is placed where the package closing brace should be.

I haven't looked at what the change to the existing emacs-mode was in git, but the one in 0.24.4 worked properly (although presumably there was some fix that I was not running into?)

04/29/08 20:18:24 changed by rra

Hm, this too works correctly for me in Emacs 22. After reindenting with Tab, I get:

class foo {

   package {
     "foo":
       ensure => installed;
   }
}

which is what I would expect. I'm not sure what would change between Emacs 21 and Emacs 22 that would cause this to change.

The changes since 0.24.4 shouldn't have affected this; they were targetted at other problems.

I may have to install Emacs 21 somewhere to try to figure out what's going on. I'll try to get to this before too long.

04/29/08 21:10:49 changed by micah

The behavior I described is the same for me with emacs22 or emacs21. Using the latest in debian sid.

Perhaps I have not installed this properly? I just edited the puppet-mode.el that is in /usr/share/emacs/site-lisp with the changes you describe above. I didn't do anything other than reload emacs.

04/29/08 23:27:06 changed by rra

Is that the only contents of the file? It's possible that it's getting confused by something earlier in the file.

04/29/08 23:44:47 changed by micah

Nope, thats the only contents in the file... I created foo.pp with only those contents.

05/03/08 05:48:39 changed by rra

Sorry, I was just very confused. Your test case was of course fine and exposed a lack of sophistication in the indenting rules.

I have fixed this, as well as a whole pile of other things, in my repository. You can get the new version from the emacs-mode branch of the repo at http://archives.eyrie.org/software/git/puppet.git -- please try and let me know if it now works for you.

[31a606de8628fafbde7be7ff54a0813f61cb2c44] is the initial commit to fix the Emacs 21 problem. [6e69c393f8f6623a69d268fd63d5d0ba1c5b8fcf] is the subsequent fix for this problem and many others.

05/03/08 22:57:20 changed by micah

I tried out these changes, and although things have certainly improved, its not working correctly.

Take for example:

class foo {

  package {
    "somepackage":
  ensure => installed;

"someotherpackage":
ensure => installed
  }
}

There are two things to note here:

1. The ensure line is not indented, but the package name is. The way it was before the package name was indented, and then the ensure line (and all other related lines) were also indented, but one level deeper to distinguish them as sub-components of the parent(s)

2. A second package isn't indented at all

05/03/08 22:58:48 changed by micah

Another example which illustrates a tertiary issue:

  service {
    "dsyslog":
  name => "dsyslog",
ensure => running,
enable => true,
hasstatus => true,
hasrestart => true,
require => Package["dsyslog"]
  }

The indentation here is not right.

05/04/08 04:24:43 changed by rra

Bleh, sorry, this is another variation of a bug that I fought with for most of Friday, and I still missed a few more places where it was a problem. [^...] matches newlines and will look right past the current line.

This should now be fixed now and your examples now work for me. The new version is in the same repository as above, and the new commit that fixes this is [73c7aa5541fb24096193205996cabcd8404a587f].

05/04/08 05:11:06 changed by micah

  • owner changed from rra to luke.
  • status changed from assigned to new.
  • stage changed from Needs more info to Ready for checkin.

The latest commit works for me as well. I think this is ready for checkin.

05/04/08 05:18:26 changed by rra

Thank you for all the testing, and I'm sorry about the problems. This is the first Emacs mode that I did serious work on, so I'm sort of learning as I go.

05/09/08 22:50:38 changed by micah

Thanks for the improvements to the puppet mode that you've made, I dont mind the testing, or the problems, its all part of the process!

On that note, I did notice one minor thing that isn't huge, if you try to do the same with a custom module block:

class test {

  case $hostname {
    "puffin": {
      backupninja::rdiff {
        "rdiff_puffin":
          type => 'remote',
          directory => '/crypta/puffin',
          host => 'kakapo-pn',
          user => 'backpuffin',
          keep => 60,
          include => ['/var/backups', '/var/lib/dpkg/status', '/var/spool/cron/crontabs', '/etc', '/root'];
      }
}
}
}

Notice how the closing braces are all on the left-hand side?

05/09/08 22:50:57 changed by micah

  • owner changed from luke to micah.
  • stage changed from Ready for checkin to Accepted.

05/10/08 00:24:35 changed by rra

Thanks, this is fixed in [0ffcf845ad4477851a7535f881082b1cb5f788bd]. And I rebased my emacs-mode branch and then realized that was a stupid idea; sorry about that. So the previous three commits, from earliest to latest, that should also still be merged are:

[286356f0a4afe1bd7baea5d8a43742e86ee0c07e] [57a60aecb7f3d28286a11c4838267f4bf16ec7c6] [0ffcf845ad4477851a7535f881082b1cb5f788bd]

05/10/08 19:40:56 changed by micah

  • owner changed from micah to luke.
  • stage changed from Accepted to Ready for checkin.
  • milestone set to 0.24.5.

Thanks for the quick fix, this resolves the nested issue nicely!

05/12/08 23:49:11 changed by luke

  • status changed from new to closed.
  • resolution set to fixed.

I have merged in all of the commits, but I didn't really know where they were on your system, so I just cherry-picked them individually.