-
Notifications
You must be signed in to change notification settings - Fork 372
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
Minimize unnecessary cursor shifting at editor, various fixes & improvements #1839
Conversation
@gsantner Not sure I will get this PR done soon. If it isn't ready by the weekend, please don't wait to release. This is a minor issue and I will be on vacation for a week :) |
app/src/main/java/net/gsantner/markor/activity/DocumentShareIntoFragment.java
Show resolved
Hide resolved
app/src/main/java/net/gsantner/markor/frontend/textview/HighlightingEditor.java
Show resolved
Hide resolved
app/src/main/java/net/gsantner/markor/frontend/textview/HighlightingEditor.java
Outdated
Show resolved
Hide resolved
app/src/main/java/net/gsantner/markor/activity/DocumentEditAndViewFragment.java
Outdated
Show resolved
Hide resolved
app/src/main/java/net/gsantner/markor/activity/DocumentEditAndViewFragment.java
Show resolved
Hide resolved
app/src/main/java/net/gsantner/markor/activity/DocumentEditAndViewFragment.java
Show resolved
Hide resolved
app/src/main/java/net/gsantner/markor/activity/DocumentEditAndViewFragment.java
Show resolved
Hide resolved
app/src/main/java/net/gsantner/markor/frontend/textview/HighlightingEditor.java
Outdated
Show resolved
Hide resolved
app/src/main/java/net/gsantner/markor/frontend/textview/HighlightingEditor.java
Outdated
Show resolved
Hide resolved
app/src/main/java/net/gsantner/markor/frontend/textview/HighlightingEditor.java
Show resolved
Hide resolved
app/src/main/java/net/gsantner/markor/frontend/textview/TextViewUtils.java
Outdated
Show resolved
Hide resolved
app/src/main/java/net/gsantner/markor/frontend/textview/HighlightingEditor.java
Show resolved
Hide resolved
I have the file with the long line and my test files. All work satisfactorily.
H
Oct. 7, 2022 11:25:03 Gregor Santner ***@***.***>:
… ***@***.**** commented on this pull request.
----------------------------------------
In app/src/main/java/net/gsantner/markor/frontend/textview/HighlightingEditor.java[#1839 (comment)]:
> final int[] newHlRegion = hlRegion(_hlRect); // Compute this _before_ clear
- try {
- beginBatchEdit();
- blockBringPointIntoView(); // Hack to block bring point into view
do you need some testfiles again or do you have it all around?
—
Reply to this email directly, view it on GitHub[#1839 (comment)], or unsubscribe[https://github.com/notifications/unsubscribe-auth/AAOZ3TDAEUTPFDRQ3ZFOQFDWCBTH5ANCNFSM6AAAAAAQJGZZGY].
You are receiving this because you were mentioned.[Tracking image][https://github.com/notifications/beacon/AAOZ3TG37ENERH3LENDKHETWCBTH5A5CNFSM6AAAAAAQJGZZG2WGG33NNVSW45C7OR4XAZNRKB2WY3CSMVYXKZLTORJGK5TJMV32UY3PNVWWK3TUL5UWJTSDUWJRC.gif]
|
Imo this is ready to go |
@@ -982,9 +983,16 @@ public String getMimeType(final Context context, String uri) { | |||
mimeType = GsFileUtils.getMimeType(new File(uri)); | |||
} | |||
|
|||
// Detect extensions which open in this app at plain text | |||
// Done only if all other tests fail | |||
if (GsTextUtils.isNullOrEmpty(mimeType) && new AppSettings().init(context).isExtOpenWithThisApp(ext)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Markor AppSettings shouldn't be in GsContextUtils, I will check what I can do here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we can create an isPlainText
file function somewhere in markor. But I think we have too many mime type / isTextFile functions rn, each with different semantics.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How did this check get here? I recall the ext check to be explicitley there where we do also the other extension-to-type association. At converters...and this one being in PlaintextConverter.
Btw I can also think of a other way where we need no additional extension code at utils, but still resolving works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added it to make sure snippets and templates showed all possible files.
We could probably just move it out to the actual snippet and template logic for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is FormatRegistry.isFileSupported() what you look for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that will do. Pushed the change.
I also think we should work on unifying the various mime type / file type / isTextFile functions. There are way too many now.
is it ready for next check? / merge |
Yes. It should be done. |
@gsantner I have fixed the way we set default preview state here. Current behavior is:
Otherwise we use the last state |
Am seeing this weird effect - If you put your cursor on the last line in todo.txt and open a dialog (say search) and then the dialog is dismissed (by cancel etc), the textview scrolls away from the line.
Doesn't seem to happen if we select a line which is not the last.