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

Jayemar/include dependency on f #67

Merged

Conversation

jayemar
Copy link
Collaborator

@jayemar jayemar commented Aug 12, 2023

Previous changes made use of the f package, but this package was not included as a dependency, so this includes the dependency.

@jayemar
Copy link
Collaborator Author

jayemar commented Aug 12, 2023

@licht1stein Tests against Emacs 27.2 have started failing for me due to a difference between the directory-files function used in Emacs 27.2 versus 28.1. In 27.2 only 4 parameters are allowed for this function, whereas 28.2 allows 5 parameters. We're using elgrep which makes use of directory-files but passes 5 parameters which works fine in 28.2 but causes failures in 27.2, thereby causing an error when we use obsidian--grep.

Are you aware of any changes that may have caused this? I understand why this is failing, but not why it's only started failing now and hasn't always been failing. I'll keep digging into it to try to understand what's going on, but I just wanted to make sure I wasn't missing something obvious as this kind of has the feeling like I'm doing something stupid. Thanks.

@jayemar
Copy link
Collaborator Author

jayemar commented Aug 13, 2023

Ah, I found the issue. There was a change to the elgrep package on August 10th that changed the way the function elgrep-directory-files calls directory-files. Unfortunately, the change calls directory-files with 5 arguments which essentially makes it incompatible with Emacs 27 which only supports 4 arguments for directory-files. Have to think a bit about how best to handle this.

@jayemar
Copy link
Collaborator Author

jayemar commented Aug 13, 2023

I think this can be fixed by advising the function directory-files. I opened a pull request for this change:
#68

@licht1stein
Copy link
Owner

Merged #68

@jayemar jayemar marked this pull request as ready for review September 22, 2023 15:29
@jayemar
Copy link
Collaborator Author

jayemar commented Sep 22, 2023

This branch fixes a bunch of lint warnings that were left around mostly because of my recent changes. This hopefully makes debugging with eldev easier with fewer warning messages sprinkled in.

@licht1stein licht1stein merged commit d02c3e7 into licht1stein:master Oct 10, 2023
5 checks passed
@jayemar jayemar deleted the jayemar/include-dependency-on-f branch October 11, 2023 15:38
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