licht1stein / obsidian.el

Obsidian Notes for Emacs
GNU General Public License v3.0
362 stars 27 forks source link

Jayemar/include dependency on f #67

Closed jayemar closed 11 months ago

jayemar commented 1 year ago

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

jayemar commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

I think this can be fixed by advising the function directory-files. I opened a pull request for this change: https://github.com/licht1stein/obsidian.el/pull/68

licht1stein commented 1 year ago

Merged #68

jayemar commented 12 months ago

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.