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

Cannot find existing markdown files outside inbox #72

Closed
LA-Toth opened this issue Sep 21, 2023 · 8 comments
Closed

Cannot find existing markdown files outside inbox #72

LA-Toth opened this issue Sep 21, 2023 · 8 comments

Comments

@LA-Toth
Copy link
Contributor

LA-Toth commented Sep 21, 2023

While I worked on issue #70, I tried to figure out why it can't find the existing markdown file with or without my changes.

The obsidian-find-file tries to match the file from all-files coming from the cache, but fails on this line:

  (matches (obsidian--match-files f all-files))

And that was the strangely working function:

(defun obsidian--file-relative-name (f)
  "Take file name F and return relative path for `obsidian-directory'."
  (message "f   %s" f)
  (message "od  %s" obsidian-directory)
  (message "frn %s" (file-relative-name f obsidian-directory))
  (file-relative-name f obsidian-directory))

What I expected is to have a generic idea what should be shown for the actually matching file, but strangely it thinks that the file is in the inbox folder. Visiting a wiki link the code iterates on all files, so for instance the moc.md in the MY_OBSIDIAN_FOLDER produces:

f   moc.md
od  /home/user/MY_OBSIDIAN_FOLDER
frn 000 Inbox/moc.md

The reason behind that is the behavior of file-relative-name. If the first parameter, f is a relative path, like in my case, it prefixes with the current directory (based on my reproduction in **scratch** buffer), so if the current directory is the inbox, the actual filename will be /home/user/MY_OBSIDIAN_FOLDER/000 Inbox/moc.md instead of /home/user/MY_OBSIDIAN_FOLDER/moc.md, this is why the result is useless. And obviously if the file is already in the inbox, like, e.g. 000 Inbox/my-idea.md, the result is 000 Inbox/000 Inbox/moc.md.

So this function should not be called here.

@LA-Toth
Copy link
Contributor Author

LA-Toth commented Sep 21, 2023

Oh, now I see. When I first tried obsidian.el, it worked for me. Fixing Issue #64 broke this. When I reverted commit 3fbd5dc, obsidian.el started to work again.

@jayemar jayemar mentioned this issue Sep 25, 2023
@jayemar
Copy link
Collaborator

jayemar commented Sep 25, 2023

I'm running into this issue, too. I just made a comment on the pull request that introduced that code to reference this issue.

@mastro35
Copy link
Collaborator

mastro35 commented Oct 1, 2023

Sorry if I answer just now, I had a difficult week actually…
The problem here seems to be the function obsidian--file-relative-name, don’t it?

I mean, it was incidental that was working before I fixed #64, it worked just because the obsidian--match-files was extracting way more files than it was necessary (all the files ending with a given parameter) and not just the files matching the filename as the function name suggests.

I’ll try to help finding a solution, can you please send me an example of the bug you are experiencing? I mean, if you could create a test vault where the problem occurs it will be great.

@LA-Toth
Copy link
Contributor Author

LA-Toth commented Oct 1, 2023

I'm not sure if obsidian--file-relative-name itself behaves abnormally.
obsidian--file-relative-name calls this function since your change, unfortunately with relative path, so the called obsidian--file-relative-name assumes that the filename is relative to the current working directory.

The original code was:

(-filter (lambda (el) (s-ends-with-p f el)) all-files))

It should become something like:

(-filter (lambda (el) (or (s-equals-p f el) (s-ends-with-p (concat "/" f) el))) all-files))

So either there is an actual match or one or more that ends with the actual file name.
Like in #64, in case of aaa.md and not-the-aaa.md, and the link [[aaa]], it finds only the aaa.md.

My use case:
have a foo/bar.md and foo/baz.md and in foo/bar.md there is [[baz]].
Normally it opens foo/baz.md, but since #64 it opens inbox/baz.md.

@mastro35
Copy link
Collaborator

mastro35 commented Oct 2, 2023

I keep not understanding why you think it is correct that obsidian--file-relative-name assumes that the filename is relative to the current working directory if you have it in your obsidian vault...

However, your solution seems fine to me, go ahead and create a pull request.

LA-Toth added a commit to LA-Toth/obsidian.el that referenced this issue Oct 2, 2023
When issue licht1stein#64 was opened, obsidian--match-files was lazy,
only the suffix was checked, so it found for 'aa.md' both
'aa.md' and 'baa.md'. Its solution broke the relative file name
usage (e.g. [[aa.md]] was "found" in the inbox).

To cover both use case the match needs to be either exact
(relative to the vault root), or partial, where the searched
file name prefixed with '/' is searched with s-ends-with-p.

Fixes issue licht1stein#72.
@LA-Toth
Copy link
Contributor Author

LA-Toth commented Oct 2, 2023

I keep not understanding why you think it is correct that obsidian--file-relative-name assumes that the filename is relative to the current working directory if you have it in your obsidian vault...

I just wrote that if it is not called from obsidian--match-files, it may work well, I can't tell because I didn't check its other calls.

LA-Toth added a commit to LA-Toth/obsidian.el that referenced this issue Oct 2, 2023
When issue licht1stein#64 was opened, obsidian--match-files was lazy,
only the suffix was checked, so it found for 'aa.md' both
'aa.md' and 'baa.md'. Its solution broke the relative file name
usage (e.g. [[aa.md]] was "found" in the inbox).

To cover both use case the match needs to be either exact
(relative to the vault root), or partial, where the searched
file name prefixed with '/' is searched with s-ends-with-p.

Fixes issue licht1stein#72.
LA-Toth added a commit to LA-Toth/obsidian.el that referenced this issue Oct 19, 2023
When issue licht1stein#64 was opened, obsidian--match-files was lazy,
only the suffix was checked, so it found for 'aa.md' both
'aa.md' and 'baa.md'. Its solution broke the relative file name
usage (e.g. [[aa.md]] was "found" in the inbox).

To cover both use case the match needs to be either exact
(relative to the vault root), or partial, where the searched
file name prefixed with '/' is searched with s-ends-with-p.

Fixes issue licht1stein#72.
@gleitz
Copy link

gleitz commented Nov 3, 2023

I agree that this PR needs to be merged. More info at #73 (comment)

licht1stein pushed a commit that referenced this issue Nov 5, 2023
When issue #64 was opened, obsidian--match-files was lazy,
only the suffix was checked, so it found for 'aa.md' both
'aa.md' and 'baa.md'. Its solution broke the relative file name
usage (e.g. [[aa.md]] was "found" in the inbox).

To cover both use case the match needs to be either exact
(relative to the vault root), or partial, where the searched
file name prefixed with '/' is searched with s-ends-with-p.

Fixes issue #72.
@LA-Toth
Copy link
Contributor Author

LA-Toth commented Dec 7, 2023

I think this can be closed as it's fixed.

@LA-Toth LA-Toth closed this as completed Dec 7, 2023
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

No branches or pull requests

4 participants