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

Support opening relative links #5

Closed
amake opened this issue Jun 7, 2020 · 12 comments
Closed

Support opening relative links #5

amake opened this issue Jun 7, 2020 · 12 comments
Labels
enhancement New feature or request

Comments

@amake
Copy link
Owner

amake commented Jun 7, 2020

It would be nice to support opening relative file links like [[./other_file.org][related text]].

Issues:

  • Is it even possible to resolve a URI relative to one that we have obtained permission for?
    • Maybe we can prompt for permissions, e.g. show the file picker with the target pre-selected?
  • How should the file open?
    • Push a new route on top of the current?
    • Or close the current and open the new file?
@amake amake added the enhancement New feature or request label Jun 7, 2020
@alexeyre
Copy link

alexeyre commented Jun 24, 2020

I know apps like Syncthing use the new scoped-storage API on android to request access to a whole folder, so that's an option for addressing issue 1, for android at least, I don't know anything about iOS development...
imho the file should close the current file and open the new one in it's place, rather than trying to implement some kind of slide-over system, but that's just my two-cents.

@fgilbert68
Copy link

fgilbert68 commented Nov 29, 2020

I would really like this. I heavily rely on links like [[other_file.org::*Some outline][related text]] to switch between ideas and the Org file where they are written.
For me, the natural way would be to close the current file and open the new one; displaying a "narrow" view if the link includes an outline.
It would be a bonus if we can navigate back to where the link was, as we can do using "C-c &" in Emacs.

I'm not sure I understand the URI issue (but I don't know anything about the apparently very complex world of Android permissions). All my org files are in the same directory on the SD card (copied there using rsync through a davfs mount...), and if this is the case (as you suggested by writing [[./other_file.org]]), no extra permission should be needed?

@amake
Copy link
Owner Author

amake commented Dec 2, 2020

All my org files are in the same directory on the SD card (copied there using rsync through a davfs mount...), and if this is the case (as you suggested by writing [[./other_file.org]]), no extra permission should be needed?

Right now Orgro can only obtain permissions to individual files. So to access a separate file I would need to make you choose it from a picker. If I can preselect the file that might not be so bad. If I cache the access ID then you'd only have to choose it once.

But then you're still choosing each new file.

I'm not sure if obtaining permission to a directory would let me access all of its children. It might. But then there is a new concept of "choosing a directory" to somehow include in the UI and make it clear how/why you need to do that. That might require redoing all of the file-opening workflow code.

So this ticket is stuck waiting for me to have time to investigate all of this.

@mcubrilo

This comment has been minimized.

@alensiljak
Copy link

alensiljak commented Mar 10, 2021

Very briefly, I can confirm that Flutter on Android can obtain permissions for a folder on external storage. Just recently I worked on the related code in GitJournal. This allows opening any file within the hierarchy below the selected location.

I'd agree that a more-encompassing strategy regarding file-opening would be worthwhile. I'm using Orgro as the viewer for the files edited by Orgzly, located in a local repository (folder). The folder tree is synchronized via rclone or syncthing.

Perhaps an option to "add a folder" could simply display all the .org files within?
The issue with the current file-based permission from the content provider is that the permission is void as soon as the file/content is edited. After each edit, I need to re-select the file to preview it. Hopefully I still remember where exactly it is located.
Going further, iterating through a folder tree and displaying notes (hopefully in a tree structure) would solve that issue for me completely. Probably would never have to select another file in Orgro again. This would, naturally, require refreshing the list occasionally (automatically).

@amake
Copy link
Owner Author

amake commented Mar 19, 2021

For #37 I've hit upon what I consider a plausible UI for requesting document-specific access permissions, so this is partially unblocked.

My idea is to detect at open time whether there are relative links, and to compute the common root directory.

  • If we know we have read access to that directory, then we do nothing as the links are already resolvable.

  • If we don't have read access, we show a banner at the top of the document notifying the user:

    This document contains relative links. Allow access?
    [Dismiss] [Allow]
    

    Tap Allow to show a directory picker (hopefully) already navigated to the correct directory. If the user selects the directory, store the persistent bookmark for future use so that the banner doesn't need to be shown again.

Support for #22 falls out of this scheme plus #37.

Remaining issues

Getting permissions

I use file_picker_writable to get persistent access to files, but this plugin doesn't allow selecting directories. So I either need to add the functionality to that plugin, or write my own (see hpoul/file_picker_writable#16).

Resolving relative path

Resolving a relative path against a security-scoped directory URL might not be super hard, but it at least seems like it will require a round-trip to the native layer so it is non-trivial.

Handling permissions

Directory permissions should be persisted so that the user doesn't get asked every time.

Given a set of existing permissions and a set of URLs, it might be non-trivial to compute whether we need to ask for additional permissions.

Permissions may expire or otherwise become temporarily or permanently unusable (switching users in Google Drive); figuring out how to cull them might be important.

Error handling

  • How to handle links that aren't resolvable
  • How to handle persisted permissions that have become stale
  • How to handle scenarios where the base URL is not known (opening via intent)

Rejected ideas

I don't want to change the topmost file-picking UI to introduce the concept of directories, or implement some sort of generalized directory tree browser.

@alensiljak
Copy link

alensiljak commented Mar 19, 2021

I don't want to change the topmost file-picking UI to introduce the concept of directories, or implement some sort of generalized directory tree browser.

Interesting, since you mentioned you want to stick to the original OrgMode features yet here you reject the most basic one? OrgMode is defacto a file-based, therefore directory-based, system of notes. Why the disconnect?

I'm asking because it seems that most of the open issues would be resolved by implementing a folder-based structure.

@amake
Copy link
Owner Author

amake commented Mar 22, 2021

OrgMode is defacto a file-based, therefore directory-based, system of notes. Why the disconnect?

I disagree with this premise. Org Mode is buffer-based. Buffers are often backed by files, but don't have to be. Links (relative and otherwise) can point to files, but don't have to. You could have Org Mode function exactly as-is but have all the data come out of a database, or be fetched over the network, or really anything, as long as the links resolve by some mechanism.

There is nothing fundamental to Org Mode about either files or directories.

I'm asking because it seems that most of the open issues would be resolved by implementing a folder-based structure.

  • The OS already has a file/directory browser that the user already has to interact with; mine would be reinventing the wheel (probably poorly)

  • Making an in-app directory browser doesn't solve the issue.

    Even if I make the user choose a directory and then a file, thus ensuring access to the filesystem tree containing the file, the file may reference other files outside of that tree (e.g. ../foo.org). We may need to start arbitrarily far up the tree; the only way to find out is to open the file and inspect the links within.

  • There are almost certainly going to be scenarios where we have opened a file but don't have permission for anything but the file (open via intent from another app). In that case the in-app directory browser doesn't help at all.

@amake
Copy link
Owner Author

amake commented Apr 5, 2021

I have this implemented locally to some degree, but there are large limitations.

Android-specific

It appears to be a fundamental limitation of the Android Storage Access Framework that you can't resolve up the tree from a known content URI.

If you open a file /a/b/foo.org that has a relative link to ../c/bar.org, the app must prompt for access permissions to /a, but given the content URI for foo.org (obtained from a file picker) and the relative path to bar.org (obtained from the content of foo.org), there seems to be no way to get a content URI for /a or even discover the name a. This means that prompting for access has very poor UX: I can't pre-select /a for the user or even tell them to select /a; I can only say "select the directory corresponding to .. from the file foo.org.

Either that, or I have to ask for permission to manage the entire provider document tree. That doesn't seem reasonable, and even then it's not clear that all paths are readily resolvable.

While Android is particularly restrictive in this regard, and iOS is less so, by virtue of being a Flutter app I can only reasonably target the least common denominator. So iOS suffers as well.

iOS and Android

Very few content providers implement support for picking a directory (ACTION_OPEN_DOCUMENT_TREE on Android; kUTTypeFolder on iOS). The only ones I've managed to find so far are:

  • iOS
    • Local storage
    • iCloud Storage
  • Android
    • Local storage
    • Termux
    • Seafile

(Providers that definitely don't support it include Google Drive and Dropbox.)

So opening relative links will be limited to supported providers, but I don't know of a way to programmatically determine if a provider supports it, so there will be some annoyances.

@amake
Copy link
Owner Author

amake commented Apr 26, 2021

This will be implemented in v1.18.0, which will be available for testing soon:

See here for details and caveats.

amake added a commit that referenced this issue Apr 26, 2021
@amake
Copy link
Owner Author

amake commented Apr 27, 2021

v1.18.0 is now available for testing on Test Flight and Google Play.

@amake
Copy link
Owner Author

amake commented May 4, 2021

This is implemented in v1.18.1, which is out for all platforms.

@amake amake closed this as completed May 4, 2021
amake added a commit that referenced this issue Oct 28, 2022
```
══╡ EXCEPTION CAUGHT BY SCHEDULER LIBRARY ╞═════════════════════════════════════════════════════════
The following assertion was thrown during a scheduler callback:
Layer OffsetEngineLayer was previously used as oldLayer.
Once a layer is used as oldLayer, it may not be used again. Instead, after calling one of the
SceneBuilder.push* methods and passing an oldLayer to it, use the layer returned by the method as
oldLayer in subsequent frames.
'dart:ui/compositing.dart':
Failed assertion: line 110 pos 9: '<optimized out>'

Either the assertion indicates an error in the framework itself, or we should provide substantially
more information in this error message to help you determine and fix the underlying cause.
In either case, please report this assertion by filing a bug on GitHub:
  https://github.com/flutter/flutter/issues/new?template=2_bug.md

When the exception was thrown, this was the stack:
#2      _EngineLayerWrapper._debugCheckNotUsedAsOldLayer (dart:ui/compositing.dart:110:9)
#3      SceneBuilder.addRetained.<anonymous closure>.recursivelyCheckChildrenUsedOnce (dart:ui/compositing.dart:695:21)
#4      List.forEach (dart:core-patch/growable_array.dart:416:8)
#5      SceneBuilder.addRetained.<anonymous closure>.recursivelyCheckChildrenUsedOnce (dart:ui/compositing.dart:701:18)
#6      SceneBuilder.addRetained.<anonymous closure> (dart:ui/compositing.dart:704:7)
#7      SceneBuilder.addRetained (dart:ui/compositing.dart:707:6)
#8      Layer._addToSceneWithRetainedRendering (package:flutter/src/rendering/layer.dart:671:15)
#9      ContainerLayer.addChildrenToScene (package:flutter/src/rendering/layer.dart:1284:13)
#10     OffsetLayer.addToScene (package:flutter/src/rendering/layer.dart:1421:5)
#11     Layer._addToSceneWithRetainedRendering (package:flutter/src/rendering/layer.dart:674:5)
#12     ContainerLayer.addChildrenToScene (package:flutter/src/rendering/layer.dart:1284:13)
#13     ClipRectLayer.addToScene (package:flutter/src/rendering/layer.dart:1590:5)
#14     Layer._addToSceneWithRetainedRendering (package:flutter/src/rendering/layer.dart:674:5)
#15     ContainerLayer.addChildrenToScene (package:flutter/src/rendering/layer.dart:1284:13)
#16     OffsetLayer.addToScene (package:flutter/src/rendering/layer.dart:1421:5)
#17     Layer._addToSceneWithRetainedRendering (package:flutter/src/rendering/layer.dart:674:5)
#18     ContainerLayer.addChildrenToScene (package:flutter/src/rendering/layer.dart:1284:13)
#19     OffsetLayer.addToScene (package:flutter/src/rendering/layer.dart:1421:5)
#20     Layer._addToSceneWithRetainedRendering (package:flutter/src/rendering/layer.dart:674:5)
#21     ContainerLayer.addChildrenToScene (package:flutter/src/rendering/layer.dart:1284:13)
#22     OffsetLayer.addToScene (package:flutter/src/rendering/layer.dart:1421:5)
#23     Layer._addToSceneWithRetainedRendering (package:flutter/src/rendering/layer.dart:674:5)
#24     ContainerLayer.addChildrenToScene (package:flutter/src/rendering/layer.dart:1284:13)
#25     TransformLayer.addToScene (package:flutter/src/rendering/layer.dart:1914:5)
#26     ContainerLayer.buildScene (package:flutter/src/rendering/layer.dart:1097:5)
#27     RenderView.compositeFrame (package:flutter/src/rendering/view.dart:231:37)
#28     RendererBinding.drawFrame (package:flutter/src/rendering/binding.dart:514:18)
#29     WidgetsBinding.drawFrame (package:flutter/src/widgets/binding.dart:869:13)
#30     RendererBinding._handlePersistentFrameCallback (package:flutter/src/rendering/binding.dart:375:5)
#31     SchedulerBinding._invokeFrameCallback (package:flutter/src/scheduler/binding.dart:1271:15)
#32     SchedulerBinding.handleDrawFrame (package:flutter/src/scheduler/binding.dart:1200:9)
#33     SchedulerBinding._handleDrawFrame (package:flutter/src/scheduler/binding.dart:1058:5)
#34     _invoke (dart:ui/hooks.dart:145:13)
#35     PlatformDispatcher._drawFrame (dart:ui/platform_dispatcher.dart:338:5)
#36     _drawFrame (dart:ui/hooks.dart:112:31)
(elided 2 frames from class _AssertionError)
════════════════════════════════════════════════════════════════════════════════════════════════════
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants