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.

Sign up for GitHub

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 relative links to headers inside notebooks #702

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Support relative links to headers inside notebooks #702

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
13b0d48
LinkFindTarget: make functions work with arguments
Asalle May 3, 2020
210ce4a
Match "book::notebook"-like links and follow them
Asalle May 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions app/src/main/java/com/orgzly/android/ui/main/MainActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import com.orgzly.android.db.dao.NoteDao;
import com.orgzly.android.db.entity.Book;
import com.orgzly.android.db.entity.Note;
import com.orgzly.android.db.entity.NoteView;
import com.orgzly.android.db.entity.Repo;
import com.orgzly.android.db.entity.SavedSearch;
import com.orgzly.android.prefs.AppPreferences;
Expand Down Expand Up @@ -113,6 +114,8 @@
import java.util.List;
import java.util.Set;

import kotlin.Pair;

public class MainActivity extends CommonActivity
implements
ActionModeListener,
Expand Down Expand Up @@ -397,6 +400,15 @@ private void setupViewModel() {
File file = (File) userData;
openFileIfExists(file);
}
else if (userData instanceof Pair) {
if (((Pair) userData).getFirst() instanceof Book && ((Pair) userData).getSecond() instanceof NoteView) {
NoteView noteView = (NoteView) ((Pair) userData).getSecond();
Intent intent = new Intent(AppIntent.ACTION_OPEN_NOTE);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This always opens note details. I think we want to honor user preference Settings / Notes & Notebooks / Note / Link target and perhaps open a book instead, scrolling to the note.

See how that's done for ID/CUSTOM_ID links in followLinkToNoteWithProperty.

intent.putExtra(AppIntent.EXTRA_NOTE_ID, noteView.getNote().getId());
intent.putExtra(AppIntent.EXTRA_BOOK_ID, ((Book) ((Pair) userData).getFirst()).getId());
LocalBroadcastManager.getInstance(this).sendBroadcast(intent);
}
}
}
});

Expand Down
26 changes: 21 additions & 5 deletions app/src/main/java/com/orgzly/android/usecase/LinkFindTarget.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package com.orgzly.android.usecase
import android.os.Environment
import com.orgzly.android.BookName
import com.orgzly.android.data.DataRepository
import com.orgzly.android.db.entity.Book
import com.orgzly.android.db.entity.NoteView
import java.io.File

class LinkFindTarget(val path: String) : UseCase() {
Expand All @@ -15,9 +17,23 @@ class LinkFindTarget(val path: String) : UseCase() {
}

private fun openLink(dataRepository: DataRepository, path: String): Any {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name is confusing, I don't know why I named it openLink. I'd rename it to findTarget to match the use case.

val regex = """(.*)\:\:(.*)""".toRegex()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps * can be included in the regex. Something like:

Suggested change
val regex = """(.*)\:\:(.*)""".toRegex()
val regex = """(.*)::\*(.*)""".toRegex()

If not, we need to check for it below, having in mind support for other link types in the future, like:

file:sometextfile::NNN (jump to line number)
file:projects.org::some words (text search)
file:projects.org::#custom-id (headline search)

return if (isAbsolute(path)) {
File(path)

} else if (regex.matches(path)) {
val matchResults = regex.matchEntire(path)
Comment on lines +23 to +24
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want to avoid matching twice.

val (_, notebook, heading) = matchResults!!.groupValues
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!! is generally avoided in Kotlin. A single match and checking for its result should do it.

isMaybeBook(notebook)?.let { bookName ->
dataRepository.getBook(bookName.name)?.let {
val allNotes = dataRepository.getNotes(it.name)
for (note in allNotes) {
Comment on lines +28 to +29
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See getNotesByTitle in DataRepository. We have to avoid going through all book's notes.

I'd create a new getNotesByTitle that accepts book ID in addition to the title.

if (note.note.title == heading.substring(1)) { // fist char of heading is '*'
return Pair<Book, NoteView>(it, note);
}
}
}
}
File(Environment.getExternalStorageDirectory(), path)
} else {
isMaybeBook(path)?.let { bookName ->
dataRepository.getBook(bookName.name)?.let {
Expand All @@ -29,12 +45,12 @@ class LinkFindTarget(val path: String) : UseCase() {
}
}

private fun isAbsolute(path: String): Boolean {
return path.startsWith('/')
private fun isAbsolute(bookPath: String): Boolean {
return bookPath.startsWith('/')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not consistent with the name above. I don't care much about the name itself. Perhaps just link would make the most sense?

}

private fun isMaybeBook(path: String): BookName? {
val file = File(path)
private fun isMaybeBook(bookPath: String): BookName? {
val file = File(bookPath)

return if (!hasParent(file) && BookName.isSupportedFormatFileName(file.name)) {
BookName.fromFileName(file.name)
Expand Down