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

ignored-*-p: Match against regular expressions #1177

Closed
wants to merge 9 commits into from
Closed

ignored-*-p: Match against regular expressions #1177

wants to merge 9 commits into from

Conversation

shackra
Copy link
Contributor

@shackra shackra commented Sep 19, 2017

Buffers are created by other packages that appear on projectile's buffer lists, but they always have
some variation that makes impossible including all possible name cases, thus regular expressions
come handy for this sort of cases. This commit allows *-ignored-*-p functions to work with regular
expressions.


Before submitting a PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):

  • The commits are consistent with our contribution guidelines
  • You've added tests (if possible) to cover your change(s)
  • All tests are passing (make test)
  • The new code is not generating bytecode or M-x checkdoc warnings
  • You've updated the changelog (if adding/changing user-visible functionality)
  • You've updated the readme (if adding/changing user-visible functionality)

Thanks!

Buffers are created by other packages that appear on projectile's buffer lists, but they always have
some variation that makes impossible including all possible name cases, thus regular expressions
come handy for this sort of cases. This commit allows `*-ignored-*-p` functions to work with regular
expressions.
@bbatsov
Copy link
Owner

bbatsov commented Nov 9, 2017

Seems some tests are failing.

@shackra
Copy link
Contributor Author

shackra commented Nov 11, 2017

Indeed, but maybe is because I don't know how to properly set them because the changes, when evaluated, works for me.

Am I doing the modifications on the tests correctly?

shackra added 4 commits March 17, 2018 18:51
The function was incorrectly defined, causing to fail on tests. The correct thing to do is to call
`projectile-ignored-directories` which returns a sequence.
@shackra
Copy link
Contributor Author

shackra commented Mar 18, 2018

Nice, all test passed! And I think I revised and modify them correctly.

All criticism is welcome and appreciated.

@shackra
Copy link
Contributor Author

shackra commented Mar 23, 2018

Any news? :)

@bbatsov
Copy link
Owner

bbatsov commented Mar 24, 2018

I'd also update the docstrings of the changed function to mention they're doing the checks using regular expressions and also the defcustoms holding ignored names. Apart from this the PR looks good.

@shackra
Copy link
Contributor Author

shackra commented Mar 24, 2018

Clarifications added :)

Nothing fantastic, I made the annotations about regular expressions short and simple.

@shackra
Copy link
Contributor Author

shackra commented Apr 26, 2018

what do you guys think of the PR? 😀

@@ -367,7 +367,7 @@ containing a root file."
:type '(repeat string))

(defcustom projectile-globally-unignored-files nil
"A list of files globally unignored by projectile."
"A list of files globally unignored by projectile. Regular expressions can be used."
Copy link
Owner

Choose a reason for hiding this comment

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

The second sentence should be on the next line. This applies to all the docstring changes.

(should (projectile-ignored-directory-p "/path/to/project/tmp"))
(should-not (projectile-ignored-directory-p "/path/to/project/log"))))
(noflet ((projectile-ignored-directories () '("/path/to/project/tmp" "/path/to/project/t\\.*")))
(should (projectile-ignored-directory-p "/path/to/project/tmp"))
Copy link
Owner

Choose a reason for hiding this comment

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

You broke the indentation here. (likely because you didn't require noflet and Emacs didn't know how to indent it correctly.

@bbatsov
Copy link
Owner

bbatsov commented Jul 24, 2018

Ops, I forgot about it.

Looks good overall. I'm a bit concerned that some buffer/file names might also matched the wrong thing when interpreted as a regexp (e.g. *messages*), but let's hope this won't be a practical problem.

@bbatsov
Copy link
Owner

bbatsov commented Jul 24, 2018

This also needs to rebased on top of the current master.

@shackra
Copy link
Contributor Author

shackra commented Jul 27, 2018

okay, brb

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