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

Jump to bottom

Performance improvements #1735

Merged
merged 9 commits into from
Jun 15, 2022
Merged

Performance improvements #1735

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3290554
Many perf and other changes
harshad1 Jun 5, 2022
304c892
Cleaned inputs
harshad1 Jun 5, 2022
6ba07ef
removed logging
harshad1 Jun 5, 2022
222b339
Unused imports
harshad1 Jun 5, 2022
fa09373
Removed unneeded diff
harshad1 Jun 5, 2022
86ca008
Diffs
harshad1 Jun 5, 2022
1035ba2
Fixed test
harshad1 Jun 6, 2022
bfd0106
Fixed test
harshad1 Jun 6, 2022
2f9c3f8
Bugfix when ok text == 0
harshad1 Jun 8, 2022
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
14 changes: 7 additions & 7 deletions app/src/main/java/net/gsantner/markor/activity/DocumentEditFragment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import net.gsantner.markor.util.MarkorWebViewClient;
import net.gsantner.markor.util.ShareUtil;
import net.gsantner.opoc.activity.GsFragmentBase;
import net.gsantner.opoc.android.dummy.TextWatcherDummy;
import net.gsantner.opoc.preference.FontPreferenceCompat;
import net.gsantner.opoc.ui.FilesystemViewerData;
import net.gsantner.opoc.util.ActivityUtils;
Expand Down Expand Up @@ -213,8 +214,13 @@ public void onViewCreated(@NonNull View view, Bundle savedInstanceState) {

// Set initial wrap state
initDocState();
}

final Runnable debounced = StringUtils.makeDebounced(500, () -> {
checkTextChangeState();
updateUndoRedoIconStates();
});
_hlEditor.addTextChangedListener(TextWatcherDummy.after(s -> debounced.run()));
}

@Override
public void onFragmentFirstTimeVisible() {
Expand Down Expand Up @@ -570,12 +576,6 @@ public void onFsViewerConfig(FilesystemViewerData.Options dopt) {
}
}

@OnTextChanged(value = R.id.document__fragment__edit__highlighting_editor, callback = OnTextChanged.Callback.AFTER_TEXT_CHANGED)
public void onContentEditValueChanged(CharSequence text) {
checkTextChangeState();
updateUndoRedoIconStates();
}

public void checkTextChangeState() {
final boolean isTextChanged = !_document.isContentSame(_hlEditor.getText());

Expand Down
8 changes: 1 addition & 7 deletions app/src/main/java/net/gsantner/markor/model/Document.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import net.gsantner.markor.util.AppSettings;
import net.gsantner.markor.util.ShareUtil;
import net.gsantner.opoc.util.FileUtils;
import net.gsantner.opoc.util.StringUtils;

import java.io.File;
import java.io.FileInputStream;
Expand Down Expand Up @@ -233,13 +232,8 @@ public boolean saveContent(final Context context, final CharSequence content) {
return saveContent(context, content, null, false);
}

// Doing as we don't want to convert to string or copy unless necessary
private static int trimmedLength(final CharSequence c) {
return StringUtils.getLastNonWhitespace(c, c.length()) - StringUtils.getNextNonWhitespace(c, 0);
}

public synchronized boolean saveContent(final Context context, final CharSequence content, ShareUtil shareUtil, boolean isManualSave) {
if (!isManualSave && trimmedLength(content) < ShareUtil.MIN_OVERWRITE_LENGTH) {
if (!isManualSave && TextUtils.getTrimmedLength(content) < ShareUtil.MIN_OVERWRITE_LENGTH) {
return false;
}

Expand Down
106 changes: 43 additions & 63 deletions app/src/main/java/net/gsantner/markor/ui/hleditor/TextActions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,11 +200,7 @@ private List<String> loadActionPreference(final String suffix) {
String formatKey = _activity.getResources().getString(getFormatActionsKey()) + suffix;
SharedPreferences settings = _activity.getSharedPreferences(ACTION_ORDER_PREF_NAME, Context.MODE_PRIVATE);
String combinedKeys = settings.getString(formatKey, null);
List<String> values = Collections.emptyList();
if (combinedKeys != null) {
values = new ArrayList<String>(Arrays.asList(combinedKeys.split(",")));
}
return values;
return combinedKeys != null ? Arrays.asList(combinedKeys.split(",")) : Collections.emptyList();
}

/**
Expand Down Expand Up @@ -330,10 +326,14 @@ protected void runRegularPrefixAction(final String action, final String replaceS
}

public static class ReplacePattern {
public final Pattern searchPattern;
public final Matcher matcher;
public final String replacePattern;
public final boolean replaceAll;

public boolean isSameReplace() {
return replacePattern.equals("$0");
}

/**
* Construct a ReplacePattern
*
Expand All @@ -342,11 +342,15 @@ public static class ReplacePattern {
* @param replaceAll whether to replace all or just the first
*/
public ReplacePattern(Pattern searchPattern, String replacePattern, boolean replaceAll) {
this.searchPattern = searchPattern;
this.matcher = searchPattern.matcher("");
this.replacePattern = replacePattern;
this.replaceAll = replaceAll;
}

public CharSequence replace() {
return replaceAll ? matcher.replaceAll(replacePattern) : matcher.replaceFirst(replacePattern);
}

public ReplacePattern(String searchPattern, String replacePattern, boolean replaceAll) {
this(Pattern.compile(searchPattern), replacePattern, replaceAll);
}
Expand All @@ -361,98 +365,74 @@ public ReplacePattern(String searchPattern, String replacePattern) {
}

public void runRegexReplaceAction(final ReplacePattern... patterns) {
runRegexReplaceAction(Arrays.asList(patterns), false);
runRegexReplaceAction(Arrays.asList(patterns));
}

public void runRegexReplaceAction(final List<ReplacePattern> patterns) {
runRegexReplaceAction(patterns, false);
runRegexReplaceAction(_hlEditor, patterns);
}

public void runRegexReplaceAction(final String pattern, final String replace) {
runRegexReplaceAction(Arrays.asList(new ReplacePattern(pattern, replace)), false);
}

public void runRegexReplaceAction(final List<ReplacePattern> patterns, final boolean matchAll) {
runRegexReplaceAction(_hlEditor, patterns, matchAll);
runRegexReplaceAction(Collections.singletonList(new ReplacePattern(pattern, replace)));
}

public static void runRegexReplaceAction(final EditText editor, final ReplacePattern... patterns) {
runRegexReplaceAction(editor, Arrays.asList(patterns), false);
runRegexReplaceAction(editor, Arrays.asList(patterns));
}

/**
* Runs through a sequence of regex-search-and-replace actions on each selected line.
* This function wraps _runRegexReplaceAction with a call to disable text trackers
*
* @param patterns An array of ReplacePattern
* @param matchAll Whether to stop matching subsequent ReplacePatterns after first match+replace
*/
public static void runRegexReplaceAction(final EditText editor, final List<ReplacePattern> patterns, final boolean matchAll) {
public static void runRegexReplaceAction(final EditText editor, final List<ReplacePattern> patterns) {
if (editor instanceof HighlightingEditor) {
((HighlightingEditor) editor).withAutoFormatDisabled(() -> _runRegexReplaceAction(editor, patterns, matchAll));
((HighlightingEditor) editor).withAutoFormatDisabled(() -> _runRegexReplaceAction(editor, patterns));
} else {
_runRegexReplaceAction(editor, patterns, matchAll);
_runRegexReplaceAction(editor, patterns);
}
}

private static void _runRegexReplaceAction(final EditText editor, final List<ReplacePattern> patterns, final boolean matchAll) {
private static void _runRegexReplaceAction(final EditText editor, final List<ReplacePattern> patterns) {

Editable text = editor.getText();
final int[] selection = StringUtils.getSelection(editor);
final int[] lStart = StringUtils.getLineOffsetFromIndex(text, selection[0]);
final int[] lEnd = StringUtils.getLineOffsetFromIndex(text, selection[1]);
final int[] sel = StringUtils.getSelection(editor);
final StringUtils.ChunkedEditable text = StringUtils.ChunkedEditable.wrap(editor.getText());

// Chunk if more than one line will be acted upon
text = lEnd[0] != lStart[0] ? StringUtils.ChunkedEditable.wrap(text) : text;
// Offset of selection start from text end - used to restore selection
final int selEndOffset = text.length() - sel[1];
// Offset of selection start from line end - used to restore selection
final int selStartOffset = sel[1] == sel[0] ? selEndOffset : StringUtils.getLineEnd(text, sel[0]) - sel[0];

int lineStart = StringUtils.getLineStart(text, selection[0]);
int selEnd = StringUtils.getLineEnd(text, selection[1]);
// Start of line on which sel begins
final int selStartStart = StringUtils.getLineStart(text, sel[0]);
// Number of lines we will be modifying
final int lineCount = StringUtils.countChars(text, sel[0], sel[1], '\n')[0] + 1;

while (lineStart <= selEnd && lineStart <= text.length()) {
int lineStart = selStartStart;

final int lineEnd = StringUtils.getLineEnd(text, lineStart, selEnd);
final CharSequence line = text.subSequence(lineStart, lineEnd);
for (int i = 0; i < lineCount; i++) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Much simpler and less work being done


for (final ReplacePattern pattern : patterns) {

final Matcher searcher = pattern.searchPattern.matcher(line);

// Find matched region
int matchStart = line.length();
int matchEnd = -1;
while (searcher.find()) {
matchStart = Math.min(matchStart, searcher.start());
matchEnd = Math.max(matchEnd, searcher.end());

if (!pattern.replaceAll) break; // Limit region based on search type
}
int lineEnd = StringUtils.getLineEnd(text, lineStart);
final String line = StringUtils.toString(text, lineStart, lineEnd);

if (matchEnd >= matchStart) { // Will be true iff at least one match has been found
if (!pattern.replacePattern.equals("$0")) {
final CharSequence oldRegion = line.subSequence(matchStart, matchEnd);
// Have to create a new matcher, unfortunately, as replace does not respect region
final Matcher replacer = pattern.searchPattern.matcher(oldRegion);
final String newRegion = pattern.replaceAll ? replacer.replaceAll(pattern.replacePattern) : replacer.replaceFirst(pattern.replacePattern);
text.replace(matchStart + lineStart, matchEnd + lineStart, newRegion);
// Change effective selection based on update
selEnd += newRegion.length() - oldRegion.length();
for (final ReplacePattern pattern : patterns) {
if (pattern.matcher.reset(line).find()) {
if (!pattern.isSameReplace()) {
text.replace(lineStart, lineEnd, pattern.replace());
}

if (!matchAll) break; // Exit after first match
break;
}
}

lineStart = StringUtils.getLineEnd(text, lineStart, selEnd) + 1;
lineStart = StringUtils.getLineEnd(text, lineStart) + 1;
}

// Apply changes if chunked
if (text instanceof StringUtils.ChunkedEditable) {
((StringUtils.ChunkedEditable) text).applyChanges();
}
text.applyChanges();

editor.setSelection(
StringUtils.getIndexFromLineOffset(text, lStart),
StringUtils.getIndexFromLineOffset(text, lEnd));
final int newSelEnd = text.length() - selEndOffset;
final int newSelStart = sel[0] == sel[1] ? newSelEnd : StringUtils.getLineEnd(text, selStartStart) - selStartOffset;
editor.setSelection(newSelStart, newSelEnd);
}

protected void runInlineAction(String _action) {
Expand Down
10 changes: 8 additions & 2 deletions app/src/main/java/net/gsantner/opoc/ui/SearchOrCustomTextDialog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,10 +342,16 @@ public static void showMultiChoiceDialogWithSearchFilterUI(final Activity activi
};

// Helper function to append selection count to OK button
final String okString = dopt.okButtonText != 0 ? activity.getString(dopt.okButtonText) : "";
Copy link
Collaborator Author

@harshad1 harshad1 Jun 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Unrelated fix for when okButtonText == 0.

Text actions calls are surrounded with a try-catch all, so exceptions caused here were not detected

final Button okButton = dialog.getButton(Dialog.BUTTON_POSITIVE);
final String okText = activity.getString(dopt.okButtonText) + (dopt.isMultiSelectEnabled ? " (%d)" : "");
final Callback.a0 setOkButtonState = () -> {
okButton.setText(okText.replace("%d", Integer.toString(listAdapter._selectedItems.size())));
if (okButton != null) {
if (dopt.isMultiSelectEnabled) {
okButton.setText(okString + String.format(" (%d)", listAdapter._selectedItems.size()));
} else {
okButton.setText(okString);
}
}
};

// Set ok button text initially
Expand Down
Loading