Skip to content

Use more efficient looking-at-p instead of looking-at in most places #580

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

Merged
merged 1 commit into from
Apr 17, 2015
Merged

Conversation

sergv
Copy link
Contributor

@sergv sergv commented Apr 16, 2015

Change looking-at calls into looking-at-p. The latter is more efficient as it does not clobber match-data. While the former is redundant in places where match-data is not used after call (through e.g. match-{beginning,end,string,string-no-properties} functions).

@gracjan
Copy link
Contributor

gracjan commented Apr 17, 2015

Cool! Let me review this properly.

gracjan added a commit that referenced this pull request Apr 17, 2015
Use more efficient looking-at-p instead of looking-at in most places
@gracjan gracjan merged commit bf5384a into haskell:master Apr 17, 2015
@gracjan
Copy link
Contributor

gracjan commented Apr 17, 2015

Thanks for these changes, efficiency is very important. There are other places in the code that could use similar efficiency changes. Can you do some more?

@sergv
Copy link
Contributor Author

sergv commented Apr 17, 2015

This was mostly a brief skim through the code base. I made changes whenever I was confident it won't break anything. I'll try to do more in the future.

@gracjan
Copy link
Contributor

gracjan commented Apr 18, 2015

So this one didn't go very well, including a hanging emacs session. For example haskell-decl-scan.el is full of code fragments like:

(if (looking-at "[ \t]*\\(\\sw+\\)")
    (progn
      (setq name (match-string-no-properties 1))
      (setq name-pos (match-beginning 1))
      (setq type 'class))))

So this change isn't as obvious as we thought.

@sergv
Copy link
Contributor Author

sergv commented Apr 19, 2015

Sorry. It seems I missed something, unfortunately.

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