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

Enhance line numbers #2470

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import android.webkit.JavascriptInterface;
import android.webkit.WebSettings;
import android.webkit.WebView;
import android.widget.FrameLayout;
import android.widget.HorizontalScrollView;
import android.widget.SearchView;
import android.widget.Toast;
Expand All @@ -51,6 +52,7 @@
import net.gsantner.markor.frontend.MarkorDialogFactory;
import net.gsantner.markor.frontend.filebrowser.MarkorFileBrowserFactory;
import net.gsantner.markor.frontend.textview.HighlightingEditor;
import net.gsantner.markor.frontend.textview.LineNumbersTextView;
import net.gsantner.markor.frontend.textview.TextViewUtils;
import net.gsantner.markor.model.AppSettings;
import net.gsantner.markor.model.Document;
Expand Down Expand Up @@ -91,10 +93,12 @@ public static DocumentEditAndViewFragment newInstance(final @NonNull Document do
private HighlightingEditor _hlEditor;
private WebView _webView;
private MarkorWebViewClient _webViewClient;
private FrameLayout _editorHolder;
private ViewGroup _textActionsBar;

private DraggableScrollbarScrollView _primaryScrollView;
private HorizontalScrollView _hsView;
private LineNumbersTextView _lineNumbersView;
private SearchView _menuSearchViewForViewMode;
private Document _document;
private FormatRegistry _format;
Expand All @@ -103,6 +107,7 @@ public static DocumentEditAndViewFragment newInstance(final @NonNull Document do
private MenuItem _saveMenuItem, _undoMenuItem, _redoMenuItem;
private boolean _isPreviewVisible;
private boolean _nextConvertToPrintMode = false;
private long _lineNumbersRefreshTime; // Line numbers refresh time on scroll changed


public DocumentEditAndViewFragment() {
Expand Down Expand Up @@ -132,9 +137,23 @@ public void onViewCreated(@NonNull View view, Bundle savedInstanceState) {
final Activity activity = getActivity();

_hlEditor = view.findViewById(R.id.document__fragment__edit__highlighting_editor);
_editorHolder = view.findViewById(R.id.document__fragment__edit__editor_holder);
_textActionsBar = view.findViewById(R.id.document__fragment__edit__text_actions_bar);
_webView = view.findViewById(R.id.document__fragment_view_webview);
_primaryScrollView = view.findViewById(R.id.document__fragment__edit__content_editor__scrolling_parent);
_primaryScrollView.getViewTreeObserver().addOnScrollChangedListener(() ->
{
if (_lineNumbersView.isLineNumbersEnabled()) {
final long time = System.currentTimeMillis();
if (time - _lineNumbersRefreshTime > 125) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why a time based vs a scroll based refresh like we do now?

Copy link
Contributor Author

@guanglinn guanglinn Nov 10, 2024

Choose a reason for hiding this comment

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

The time based code just reduces the refresh rate of line numbers when scrolling.

Copy link
Collaborator

Choose a reason for hiding this comment

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

But we already have a system where the line numbering only redraws after the user has scrolled by some delta (1 page iirc). IMO that is superior

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That system is called passively when the view is changed, it works well when the view is EditText, but now this view is TextView. So this code is designed to actively trigger that system to check and determine whether to draw new line numbers.

_lineNumbersRefreshTime = time;
_lineNumbersView.forceRefresh();
}
}
});
_lineNumbersView = view.findViewById(R.id.document__fragment__edit__line_numbers_view);
_lineNumbersView.setEditText(_hlEditor);
guanglinn marked this conversation as resolved.
Show resolved Hide resolved
_lineNumbersView.setLineNumbersEnabled(_appSettings.getDocumentLineNumbersEnabled(_document.path));
_cu = new MarkorContextUtils(activity);

// Using `if (_document != null)` everywhere is dangerous
Expand Down Expand Up @@ -202,7 +221,6 @@ public void onViewCreated(@NonNull View view, Bundle savedInstanceState) {
_hlEditor.setTextColor(_appSettings.getEditorForegroundColor());
_hlEditor.setGravity(_appSettings.isEditorStartEditingInCenter() ? Gravity.CENTER : Gravity.NO_GRAVITY);
_hlEditor.setHighlightingEnabled(_appSettings.getDocumentHighlightState(_document.path, _hlEditor.getText()));
_hlEditor.setLineNumbersEnabled(_appSettings.getDocumentLineNumbersEnabled(_document.path));
_hlEditor.setAutoFormatEnabled(_appSettings.getDocumentAutoFormatEnabled(_document.path));
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
// Do not need to send contents to accessibility
Expand Down Expand Up @@ -286,7 +304,7 @@ public void onPause() {
_appSettings.setDocumentPreviewState(_document.path, _isPreviewVisible);
_appSettings.setLastEditPosition(_document.path, TextViewUtils.getSelection(_hlEditor)[0]);

if(_document.path.equals(_appSettings.getTodoFile().getAbsolutePath())){
if (_document.path.equals(_appSettings.getTodoFile().getAbsolutePath())) {
TodoWidgetProvider.updateTodoWidgets();
}
super.onPause();
Expand Down Expand Up @@ -522,7 +540,7 @@ public boolean onOptionsItemSelected(@NonNull final MenuItem item) {
if (saveDocument(false)) {
TextConverterBase converter = FormatRegistry.getFormat(_document.getFormat(), activity, _document).getConverter();
_cu.shareText(getActivity(),
converter.convertMarkup(getTextString(), getActivity(), false, _hlEditor.isLineNumbersEnabled(), _document.file),
converter.convertMarkup(getTextString(), getActivity(), false, _lineNumbersView.isLineNumbersEnabled(), _document.file),
"text/" + (item.getItemId() == R.id.action_share_html ? "html" : "plain")
);
}
Expand Down Expand Up @@ -611,9 +629,9 @@ public void onFsViewerConfig(GsFileBrowserOptions.Options dopt) {
return true;
}
case R.id.action_line_numbers: {
final boolean newState = !_hlEditor.isLineNumbersEnabled();
final boolean newState = !_lineNumbersView.isLineNumbersEnabled();
_appSettings.setDocumentLineNumbersEnabled(_document.path, newState);
_hlEditor.setLineNumbersEnabled(newState);
_lineNumbersView.setLineNumbersEnabled(newState);
updateMenuToggleStates(0);
return true;
}
Expand Down Expand Up @@ -742,7 +760,7 @@ private void updateMenuToggleStates(final int selectedFormatActionId) {
mi.setChecked(_hlEditor.getHighlightingEnabled());
}
if ((mi = _fragmentMenu.findItem(R.id.action_line_numbers)) != null) {
mi.setChecked(_hlEditor.isLineNumbersEnabled());
mi.setChecked(_lineNumbersView.isLineNumbersEnabled());
}
if ((mi = _fragmentMenu.findItem(R.id.action_enable_auto_format)) != null) {
mi.setChecked(_hlEditor.getAutoFormatEnabled());
Expand All @@ -760,19 +778,17 @@ private void updateMenuToggleStates(final int selectedFormatActionId) {
}

private boolean isWrapped() {
return _hsView == null || (_hlEditor.getParent() == _primaryScrollView);
return _hsView == null || (_hlEditor.getParent() == _editorHolder);
}

private void setHorizontalScrollMode(final boolean wrap) {
final Context context = getContext();
if (context != null && _hlEditor != null && isWrapped() != wrap) {

final int[] sel = TextViewUtils.getSelection(_hlEditor);

final boolean hlEnabled = _hlEditor.getHighlightingEnabled();
_hlEditor.setHighlightingEnabled(false);

_primaryScrollView.removeAllViews();
_editorHolder.removeAllViews();
if (_hsView != null) {
_hsView.removeAllViews();
}
Expand All @@ -782,15 +798,14 @@ private void setHorizontalScrollMode(final boolean wrap) {
_hsView.setFillViewport(true);
}
_hsView.addView(_hlEditor);
_primaryScrollView.addView(_hsView);
_editorHolder.addView(_hsView);
} else {
_primaryScrollView.addView(_hlEditor);
_editorHolder.addView(_hlEditor);
}

_hlEditor.setHighlightingEnabled(hlEnabled);

// Run after layout() of immediate parent completes
(wrap ? _primaryScrollView : _hsView).post(() -> TextViewUtils.setSelectionAndShow(_hlEditor, sel));
(wrap ? _editorHolder : _hsView).post(() -> TextViewUtils.setSelectionAndShow(_hlEditor, sel));
}
}

Expand Down Expand Up @@ -862,7 +877,7 @@ private boolean isDisplayedAtMainActivity() {
}

public void updateViewModeText() {
_format.getConverter().convertMarkupShowInWebView(_document, getTextString(), getActivity(), _webView, _nextConvertToPrintMode, _hlEditor.isLineNumbersEnabled());
_format.getConverter().convertMarkupShowInWebView(_document, getTextString(), getActivity(), _webView, _nextConvertToPrintMode, _lineNumbersView.isLineNumbersEnabled());
}

public void setViewModeVisibility(final boolean show) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@
import net.gsantner.markor.ApplicationObject;
import net.gsantner.markor.activity.MainActivity;
import net.gsantner.markor.model.AppSettings;
import net.gsantner.markor.util.TextCasingUtils;
import net.gsantner.opoc.format.GsTextUtils;
import net.gsantner.opoc.wrapper.GsCallback;
import net.gsantner.opoc.wrapper.GsTextWatcherAdapter;
import net.gsantner.markor.util.TextCasingUtils;

import java.util.Objects;
import java.util.concurrent.ExecutorService;
Expand Down Expand Up @@ -65,7 +65,6 @@ public class HighlightingEditor extends AppCompatEditText {
private TextWatcher _autoFormatModifier;
private boolean _autoFormatEnabled;
private boolean _saveInstanceState = true;
private final LineNumbersDrawer _lineNumbersDrawer = new LineNumbersDrawer(this);
private final ExecutorService executor = new ThreadPoolExecutor(0, 3, 60, TimeUnit.SECONDS, new SynchronousQueue<>());
private final AtomicBoolean _textUnchangedWhileHighlighting = new AtomicBoolean(true);

Expand Down Expand Up @@ -115,17 +114,12 @@ public void afterTextChanged(final Editable s) {

@Override
public boolean onPreDraw() {
_lineNumbersDrawer.setTextSize(getTextSize());
return super.onPreDraw();
}

@Override
protected void onDraw(Canvas canvas) {
super.onDraw(canvas);

if (_numEnabled) {
_lineNumbersDrawer.draw(canvas);
}
}

// Highlighting
Expand Down Expand Up @@ -253,23 +247,6 @@ public boolean setHighlightingEnabled(final boolean enable) {
return prev;
}

public boolean isLineNumbersEnabled() {
return _numEnabled;
}

public void setLineNumbersEnabled(final boolean enable) {
if (enable ^ _numEnabled) {
post(this::invalidate);
}
_numEnabled = enable;
if (_numEnabled) {
_lineNumbersDrawer.startLineTracking();
} else {
_lineNumbersDrawer.reset();
_lineNumbersDrawer.stopLineTracking();
}
}

// Region to highlight
private int[] hlRegion() {
if (_isDynamicHighlightingEnabled) {
Expand Down Expand Up @@ -553,174 +530,4 @@ public int moveCursorToBeginOfLine(int offset) {
public boolean indexesValid(int... indexes) {
return GsTextUtils.inRange(0, length(), indexes);
}

static class LineNumbersDrawer {

private final AppCompatEditText _editor;
private final Paint _paint = new Paint();

private final int _defaultPaddingLeft;
private static final int LINE_NUMBER_PADDING_LEFT = 18;
private static final int LINE_NUMBER_PADDING_RIGHT = 12;

private final Rect _visibleArea = new Rect();
private final Rect _lineNumbersArea = new Rect();

private int _numberX;
private int _gutterX;
private int _maxNumber = 1; // to gauge gutter width
private int _maxNumberDigits;
private float _oldTextSize;
private final int[] _startLine = {0, 1}; // {line index, actual line number}

private final GsTextWatcherAdapter _lineTrackingWatcher = new GsTextWatcherAdapter() {
@Override
public void beforeTextChanged(CharSequence s, int start, int count, int after) {
_maxNumber -= GsTextUtils.countChar(s, start, start + count, '\n');
}

@Override
public void onTextChanged(CharSequence s, int start, int before, int count) {
_maxNumber += GsTextUtils.countChar(s, start, start + count, '\n');
}
};

public LineNumbersDrawer(final AppCompatEditText editor) {
_editor = editor;
_paint.setColor(0xFF999999);
_paint.setTextAlign(Paint.Align.RIGHT);
_defaultPaddingLeft = editor.getPaddingLeft();
}

public void setTextSize(final float textSize) {
_paint.setTextSize(textSize);
}

public boolean isTextSizeChanged() {
if (_paint.getTextSize() == _oldTextSize) {
return false;
} else {
_oldTextSize = _paint.getTextSize();
return true;
}
}

public boolean isMaxNumberDigitsChanged() {
final int oldDigits = _maxNumberDigits;

if (_maxNumber < 10) {
_maxNumberDigits = 1;
} else if (_maxNumber < 100) {
_maxNumberDigits = 2;
} else if (_maxNumber < 1000) {
_maxNumberDigits = 3;
} else if (_maxNumber < 10000) {
_maxNumberDigits = 4;
} else {
_maxNumberDigits = 5;
}
return _maxNumberDigits != oldDigits;
}

public boolean isOutOfLineNumbersArea() {
final int margin = (int) (_visibleArea.height() * 0.5f);
final int top = _visibleArea.top - margin;
final int bottom = _visibleArea.bottom + margin;

if (top < _lineNumbersArea.top || bottom > _lineNumbersArea.bottom) {
// Reset line numbers area
// height of line numbers area = (1.5 + 1 + 1.5) * height of visible area
_lineNumbersArea.top = top - _visibleArea.height();
_lineNumbersArea.bottom = bottom + _visibleArea.height();
return true;
} else {
return false;
}
}

public void startLineTracking() {
_editor.removeTextChangedListener(_lineTrackingWatcher);
_maxNumber = 1;
final CharSequence text = _editor.getText();
if (text != null) {
_maxNumber += GsTextUtils.countChar(text, 0, text.length(), '\n');
}
_editor.addTextChangedListener(_lineTrackingWatcher);
}

public void stopLineTracking() {
_editor.removeTextChangedListener(_lineTrackingWatcher);
}

/**
* Draw line numbers.
*
* @param canvas The canvas on which the line numbers will be drawn.
*/
public void draw(final Canvas canvas) {
if (!_editor.getLocalVisibleRect(_visibleArea)) {
return;
}

final CharSequence text = _editor.getText();
final Layout layout = _editor.getLayout();
if (text == null || layout == null) {
return;
}

// If text size or the max line number of digits changed,
// update the variables and reset padding
if (isTextSizeChanged() || isMaxNumberDigitsChanged()) {
_numberX = LINE_NUMBER_PADDING_LEFT + (int) _paint.measureText(String.valueOf(_maxNumber));
_gutterX = _numberX + LINE_NUMBER_PADDING_RIGHT;
_editor.setPadding(_gutterX + 12, _editor.getPaddingTop(), _editor.getPaddingRight(), _editor.getPaddingBottom());
}

int i = _startLine[0], number = _startLine[1];
// If current visible area is out of current line numbers area,
// iterate from the first line to recalculate the start line
if (isOutOfLineNumbersArea()) {
i = 0;
number = 1;
_startLine[0] = -1;
}

// Draw border of the gutter
canvas.drawLine(_gutterX, _lineNumbersArea.top, _gutterX, _lineNumbersArea.bottom, _paint);

// Draw line numbers
final int count = layout.getLineCount();
final int offsetY = _editor.getPaddingTop();
for (; i < count; i++) {
int start;
try {
start = layout.getLineStart(i);
} catch (IndexOutOfBoundsException ex) {
break; // Even though the drawing is against count, might throw IndexOutOfBounds during drawing
}
if (start == 0 || text.charAt(start - 1) == '\n') {
final int y = layout.getLineBaseline(i);
if (y > _lineNumbersArea.bottom) {
break;
}
if (y > _lineNumbersArea.top) {
if (_startLine[0] < 0) {
_startLine[0] = i;
_startLine[1] = number;
}
canvas.drawText(String.valueOf(number), _numberX, y + offsetY, _paint);
}
number++;
}
}
}

/**
* Reset to the state without line numbers.
*/
public void reset() {
_editor.setPadding(_defaultPaddingLeft, _editor.getPaddingTop(), _editor.getPaddingRight(), _editor.getPaddingBottom());
_maxNumberDigits = 0;
}
}
}
Loading
Loading