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

Ignore emacs backup/swap files by default. #546

Merged
merged 2 commits into from Sep 6, 2021
Merged

Conversation

zw963
Copy link
Contributor

@zw963 zw963 commented Aug 19, 2021

For fix #543

@@ -34,6 +34,9 @@ class Silencer
| \.swpx
| ^4913

# Emacs backup/swap files
| (?:\.\#.+|\#.+\#)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe these should be anchored per our discussion on the Issue. And \A and \z are the best way to anchor in Ruby so as not to be tripped up by newlines (which are rare in filenames...but possible). So I think this does it:

(?:\A\.\#.|\A\#.+\#\z)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ColinDKelley , oh, i add to wrong place, i thought where i insert in regexp is around with a ^...$

for \A, \z, agree with you, but why i can't find those anchor within exists regexp? (only see some ^, $ anchor out of group)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing that those who wrote the earlier code didn't know about the Ruby trap of ^ and $ not working the same as in every other language.

I can also believe the some of the existing patterns weren't anchored at all, even when they should have been.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, so, double confirm, you want me fix those exists regexp anchor, and then add emacs pattern into it, right?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, two separate commits should make it clear:

  1. Fix the existing ^ and $ anchors to \A and \z.
  2. Add your new emacs patterns with \A and \z anchors.

Or if you'd rather keep this PR simple, we can flip the order and I can take care of (2):

  1. You add your new emacs patterns with ^ and $ anchors.
  2. [after merging this PR] I fix all the ^ and $ anchors to \A and \z.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, let me do both of them first anyway.

Copy link
Collaborator

@ColinDKelley ColinDKelley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one comment update I can take care of when I merge this PR.

Thank you!

@@ -122,7 +122,7 @@ listener.stop # stop both listening to changes and processing them

### Ignore / ignore!

`Listen` ignores some directories and extensions by default (See DEFAULT_IGNORED_DIRECTORIES and DEFAULT_IGNORED_EXTENSIONS in Listen::Silencer).
`Listen` ignores some directories and extensions by default (See DEFAULT_IGNORED_FILES and DEFAULT_IGNORED_EXTENSIONS in Listen::Silencer).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`Listen` ignores some files and extensions by default (See

@@ -157,7 +157,7 @@ All the following options can be set through the `Listen.to` after the directory

```ruby
ignore: [%r{/foo/bar}, /\.pid$/, /\.coffee$/] # Ignore a list of paths
# default: See DEFAULT_IGNORED_DIRECTORIES and DEFAULT_IGNORED_EXTENSIONS in Listen::Silencer
# default: See DEFAULT_IGNORED_FILES and DEFAULT_IGNORED_EXTENSIONS in Listen::Silencer
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for renaming this!

@ColinDKelley ColinDKelley merged commit 029d60e into guard:master Sep 6, 2021
@ColinDKelley ColinDKelley added this to the v3.7.1 milestone Jan 13, 2022
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.

Ignore emacs editor temp file.
2 participants