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

When :org-mode/insert-file-link is on links for page aliases are broken #9342

Open
1 of 2 tasks
idanov opened this issue May 9, 2023 · 4 comments · May be fixed by #10889
Open
1 of 2 tasks

When :org-mode/insert-file-link is on links for page aliases are broken #9342

idanov opened this issue May 9, 2023 · 4 comments · May be fixed by #10889

Comments

@idanov
Copy link

idanov commented May 9, 2023

Search first

  • I searched and no similar issues were found

What Happened?

When configured with :org-mode/insert-file-link true links to the same page are created differently if referred by title vs alias:

  • [[Title]] becomes [[file:../pages/Title.org][Title]]
  • [[Alias]] becomes [[file:..//Users/user/logseq-home/pages/Alias.org][Alias]]

The second link is a broken link in other programs, since when expanded, it results into /Users/user/logseq-home/Users/user/logseq-home/pages/Alias.org.

Reproduce the Bug

  1. Configure Logseq to use Org format
  2. Configure Logseq in config.edn to use :org-mode/insert-file-link? true
  3. Create an arbitrary page in Logseq
  4. Add an alias to the page as described here or here
  5. Open a new page or a journal entry
  6. Type // in there and search for the title of the page and add it in a block
  7. Type // in there and search for the alias of the page and add it in another block
  8. See the resulting links

Expected Behavior

The resulting file link for the alias should be the same as the one for the title.

Screenshots

M1 macOS 13.3, Desktop App v0.9.4

Desktop or Mobile Platform Information

No response

Additional Context

No response

Are you willing to submit a PR? If you know how to fix the bug.

  • I'm willing to submit a PR (Thank you!)
Copy link

github-actions bot commented Jan 8, 2024

Hi There! 👋

We haven't seen any activity on this issue in a while 😴, and we just wanted to make sure that it's still relevant. If you're still experiencing this issue, you might find it helpful to update to the latest version of Logseq. The latest version includes bug fixes and new features that may help to resolve this issue, and you can download it from our website. If updating to the latest version doesn't help, please let us know by adding a comment 💬. We're here to help!

If the issue has been resolved or is no longer relevant, that's great news! 🎉
We'll go ahead and close this issue to keep our backlog organized. Please note that this issue will be closed automatically in 20 days if there is no further activity. If you need more time to resolve the issue or provide more information, please just let us know by adding a comment.

Access additional Logseq 🚀 resources:

Thanks for your contributions to Logseq! If you have any other issues or feature requests, please don't hesitate to let us know. We always welcome pull requests too!

@idanov
Copy link
Author

idanov commented Jan 17, 2024

This is still an issue.

@FrederickGeek8
Copy link

FrederickGeek8 commented Jan 17, 2024

I'm working on a PR that might fix this as a side effect. We'll see... I'm currently stuck trying to get ruby installed on my MBP so I can test if I break the mobile app. I think based on what I'm seeing in your issue here and the unit test cases I wrote, it should be fine :)

Hopefully if that works out, I should hopefully have it out sometime soon (next few days?). That being said, I've only written unit tests. Now that I think about it, I should add E2E tests... I'll have to play with that.

@FrederickGeek8
Copy link

FrederickGeek8 commented Jan 20, 2024

I'm wrapping up what I think will be a draft PR. I just finished testing my changes on iOS (it took years off my life) and I have not written end-to-end tests (just unit), but I'll open the PR as WIP: and gauge if they should be written and good next steps.

My PR should solve these issues. I think there are a couple other issues that we might want to consider in the future regarding org-mode/roam aliases and Logseq, but for now given your reproduction steps it should be sufficient. The root of the problem is the incorrect parsing of file-paths due to buggy platform-specific path resolution for missing files which seems unnecessary in my testing. The non-compliant file url you are seeing ([[file:..//Users/user/logseq-home/pages/Alias.org][Alias]]) also occurs when linking to any non-existent page. It seems like Logseq handles creation of those pages files, but Org Roam will freak out (since that file path makes no sense anyways).

We can discuss later, but here are some questions I have for future development:

  1. Should the ::alias <alias> shorthand expand to the Org #+ALIAS tag?
    • The "Aliases and external links" section only gives instructions for ::alias, which still gives you hints for using it when in Org. Mashing enter will not create is as a recognized Logseq alias however.
    • #+ALIAS does not have any hints that it is a reserved keyword like ::alias does.
  2. Should the #+ALIAS: <alias> be placed in the standard Org :PROPERTIES: section?
  3. Should there be functionality to provide better integration with Org-Roam?
    • Org-Roam uses the :ROAM_ALIASES: <alias> tag in the :PROPERTIES: section instead of :ALIAS:
    • #+ROAM_ALIASES: under the :PROPERTIES: does not appear to be recognized in Org Roam in my testing.
    • :ROAM_ALIASES: in :PROPERTIES: is not recognized by Logseq, but :ALIAS: in properties is.
    • Implementation of this compatibility would need to be discussed more broadly.
      • Recognizing :ROAM_ALIASES: in Logseq seems like it would probably be relatively easy (I haven't actually checked though...). We would maybe also want to have an option for the "alias syntax" so that creation of aliases can be Org-Roam compatible (although there is no way of creating aliases non-manually anyways, like a ::alias expansion).
      • It's possible that Org-Roam could also be enabled to recognize :ALIAS: through a third-party plugin of some kind. Perhaps there is already a built-in configuration for this, I'm not sure. It's honestly a bit strange to me that :ALIAS: isn't support already. I'm not sure if it's reserved for other purposes...

Hopefully I can post my draft PR tonight or tomorrow. I'm happy to see that my fix should solve your issues. I have to run to a social event, but hopefully you'll see my PR soon.

👋

EDIT: I just realized that you're the person who created the org-roam-logseq.el repo that I forked and linked about (which is based off another's gist). Funny....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants