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

BUGFIX: Headline dialog and file detection #2300

Merged
merged 8 commits into from
May 25, 2024

Conversation

harshad1
Copy link
Collaborator

In this PR I make 2 changes:

  1. The way we do headline dialogs has been reworked to make it cleaner and so that it closes on selection
  2. Fix the way we detect file types so that todo.txt etc are detected correctly

@@ -88,41 +91,29 @@ public static class Format {
}
}

public static final Format[] FORMATS = new Format[]{
// Order here is used to **determine** format by it's file extension and/or content heading
public static final List<Format> FORMATS = Arrays.asList(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reordered so that the detection happens correctly

@@ -41,7 +41,7 @@ public class MarkdownActionButtons extends ActionButtonBase {

private static final Pattern WEB_URL = Pattern.compile("https?://[^\\s/$.?#].[^\\s]*");

private final Set<Integer> _disabledHeadings = new HashSet<>();
private final MarkorDialogFactory.HeadlineDialogState _headlineDialogState = new MarkorDialogFactory.HeadlineDialogState();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@guanglinn Instead of inserting arbitrary data into the intent, we create and pass around a state class for the headline dialog

public static class HeadlineDialogState {
public Set<Integer> disabledLevels = new HashSet<>();
public String searchQuery = "";
public int listPosition = -1;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@guanglinn Headline dialogs remember their position, query and the currently enabled levels

FormatRegistry.FORMAT_ASCIIDOC,
FormatRegistry.FORMAT_ORGMODE,
FormatRegistry.FORMAT_CSV
);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Only these types are available in new file dialog

dialogBuilder.setOnDismissListener((dialogInterface) -> {
// Update state
dopt.listPosition = listView.getFirstVisiblePosition();
dopt.defaultText = searchEditText.getText().toString();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Update dopt with the value when closed.

@harshad1
Copy link
Collaborator Author

This should address #2297

@harshad1
Copy link
Collaborator Author

@guanglinn Please test this PR and see if the headline dialog works as expected

I will likely make further changes in the future

@guanglinn
Copy link
Contributor

@harshad1 OK, I will test this PR in time. If any unexpected behaviors are found, I will tell you.

@guanglinn
Copy link
Contributor

@harshad1 Nothing unexpected are found yet, I think it is fine.

@harshad1 harshad1 changed the title WIP: BUGFIX: Headline dialog and file detection BUGFIX: Headline dialog and file detection May 24, 2024
@harshad1
Copy link
Collaborator Author

@gsantner I think this is good enough to go if you want to merge these fixes in.

I want to make further improvements to the headline dialog among other things but I can do that in a follow up PR.

@gsantner
Copy link
Owner

gsantner commented May 24, 2024 via email

CONVERTER_EMBEDBINARY,
CONVERTER_ORGMODE,
};
new Format(FormatRegistry.FORMAT_ORGMODE, R.string.orgmode, ".org", CONVERTER_ORGMODE),
Copy link
Owner

@gsantner gsantner May 25, 2024

Choose a reason for hiding this comment

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

Effective this is also the check order. To my understanding the last 3 have to be in order EMBEDBINARY,PLAIN,NONE, and everything else above. OrgMode is currently detected as PLAINTEXT as far I understand.

@gsantner gsantner merged commit 8bc15d7 into gsantner:master May 25, 2024
1 check passed
@gsantner gsantner linked an issue May 25, 2024 that may be closed by this pull request
4 tasks
@guanglinn
Copy link
Contributor

@harshad1

I will likely make further changes in the future.

I have also had ideas to improve the headline dialog. For example,
It is not necessary to reload the headline data every time if the document has not changed;
When we open the headline dialog, the headline list can automatically locate the first current visible headline in the document and highlight it. A reference here

toc_in_dialog.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment