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

Code Folding #12950

Merged
merged 2 commits into from
Nov 19, 2017
Merged

Code Folding #12950

merged 2 commits into from
Nov 19, 2017

Conversation

kitbdev
Copy link
Contributor

@kitbdev kitbdev commented Nov 15, 2017

Script Editor Code Folding
Fixes #2732
Allows collapsing/expanding code chunks
Code chunks are lines of text where the next line has more whitespace than the first
foldable

Implementation details:
TextEdit Text Line can now be hidden
Hidden lines are not drawn
Adds fold gutter similar to breakpoint gutter
Uses icon forward for lines that can fold and icon collapse for folded lines. Mainly because I couldn't find anything better.
Editor bool setting Text Editor/Line Numbers/Code Folding
whitespace level is the number of ' ' spaces and '\t' tabs that come before any other character. '#' comments at the beginning of the line are ignored.
Property bool hiding_enabled enables/disables all code hiding. must be enabled for all code folding features.
TextEdit set_line_hidden(int line, bool enabled) sets the hidden state of a line.
TextEdit can_fold(int line) determines whether a line can be folded. The whitespace level of the next must be higher than the first line
TextEdit fold_line(int line) folds this line. This just sets all lines following this one to hidden if they are empty or has more whitespace. trailing empty lines are unhidden.
TextEdit unfold_line(int line) unhides all lines below this line until the next nonhidden line
TextEdit is_folded(int line) returns true if this line is not hidden and the next line is hidden
Searching unfolds line with found text.
Edit menu and context menu items to fold and unfold line
Default shortcuts for fold line is Alt+Left, unfold line is Alt+Right
Smooth scrolling over hidden lines and page up/page down works (smooth scrolling down over large sections of unhidden code could be better though)
Moving a line up/down unfolds hidden section
Centering viewport to cursor takes into account hidden lines

@reduz
Copy link
Member

reduz commented Nov 15, 2017 via email

String tmp2 = tx->get_line(line2);
tx->set_line(line2, tmp);
tx->set_line(line1, tmp2);
// static void swap_lines(TextEdit *tx, int line1, int line2) {
Copy link
Member

Choose a reason for hiding this comment

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

I guess this can be removed.

@reduz
Copy link
Member

reduz commented Nov 15, 2017

@Paulb23 and others will probably want to give this a check

@neikeq
Copy link
Contributor

neikeq commented Nov 15, 2017

I think code chunks should also be determined by curly braces. This is needed for C-like languages like C# and for dictionaries in GDScript.

@blurymind
Copy link

Holly cow! :o
Been waiting to see this for years..

@NathanWarden
Copy link
Contributor

Great job, very nice :)

@Paulb23
Copy link
Member

Paulb23 commented Nov 15, 2017

Nice job!

Apologies for the wall of bullet points I don't want to scare you off :P

But some feedback after playing around with it:

  • Need some more padding between the gutter and text, can be quite hard (if not impossible) to select the start of a line.
  • The arrows should be drawn the other way around i.e. > for folded lines and v for unfolded lines
  • Any line with a '#' below is foldable even on the same indent level, i.e. the following should not be foldable:

This is a line of code



# this is a line with a '#'

  • Unfolding/folding lines should not adjust viewport to the caret.
  • Folding a line while the caret is on a line that will be hidden, should place the caret on the line folded rather then the next line below.

Bugs:

  • if the last line is folded you can enter the folded section.
  • You can still click the last line even when its folded.
  • Pressing left/right enters the folded section
  • Duplicating a folded line hides the duplicated line, but unfolds the rest of the hidden text
  • Moving a folded line up does not move foldable status with it
  • Go to next/last breakpoints on a hidden lines does not automatically unfold the line.
  • If you select a line that can be hidden, and hide it the section still persists, creating some weird behaviour.
  • Scrolling over folded lines by dragging the scroll bar does not skip the folded lines
  • And yeah smooth scrolling over folded lines is weird :P
  • Creating a new script with only the following two lines cannot be folded:
func i_cant_be_folded():
	pass

Extra:

  • Function to fold all lines as well as unfold all.
  • Support for comments (Colour regions?, then multi-line strings would be caught.)

@@ -1447,7 +1471,7 @@ void ScriptTextEditor::_color_changed(const Color &p_color) {
code_editor->get_text_edit()->set_line(color_line, new_line);
}

void ScriptTextEditor::_make_context_menu(bool p_selection, bool p_color) {
void ScriptTextEditor::_make_context_menu(bool p_selection, bool p_color, int p_fold_state) {
Copy link
Member

Choose a reason for hiding this comment

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

Would be better if this was a bool, as it can't be half folded.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed.

@@ -3731,6 +3862,8 @@ void TextEdit::_update_caches() {
cache.line_spacing = get_constant("line_spacing");
cache.row_height = cache.font->get_height() + cache.line_spacing;
cache.tab_icon = get_icon("tab");
cache.folded_icon = get_icon("Collapse", "EditorIcons");
cache.can_fold_icon = get_icon("Forward", "EditorIcons");
Copy link
Member

Choose a reason for hiding this comment

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

Could probably create new icons for code folding, or at least keep them separate by adding them here

if (!is_hiding_enabled())
return false;
if (!draw_fold_gutter)
return false;
Copy link
Member

@Paulb23 Paulb23 Nov 15, 2017

Choose a reason for hiding this comment

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

It should still be possible to fold lines even if the gutter is disabled, could draw a marker on / behind the line to denote that its folded?

void TextEdit::set_hiding_enabled(int p_enabled) {
hiding_enabled = p_enabled;
if (!hiding_enabled)
set_draw_fold_gutter(false);
Copy link
Member

Choose a reason for hiding this comment

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

Same here, hiding lines and the fold gutter should be unlinked.

@@ -325,6 +334,8 @@ class TextEdit : public Control {

int get_row_height() const;

// int _get_fold_offset(int p_line_from, int p_line_to) const;
Copy link
Member

Choose a reason for hiding this comment

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

Can be removed?

@ghost
Copy link

ghost commented Nov 16, 2017

Awesome work!

I think the icons should be swapped. It should be > for collapsed and v for expanded. (Tree uses these as well.)

(Edit: Whoops, didn't see that Paulb23's already commented on this topic.)

@BastiaanOlij
Copy link
Contributor

Love this! ditto on Noshyaar and Paulb's remark, icons should be the other way around :)

@kitbdev
Copy link
Contributor Author

kitbdev commented Nov 16, 2017

Thanks for all the great feedback!
I have rebased a commit to swap the icons and fix most of what @Paulb23 mentioned. :)
I am still working on the scrolling and selecting the last line.
There is still no support for comments or brackets either. Unindented comments now are not ignored.
Also, there is an icon added to the end of the line when it is folded, click to unfold.
I used some random tools icon for this because I couldn't find an ellipses (...) icon and didn't know what else to use.

@Zylann
Copy link
Contributor

Zylann commented Nov 16, 2017

Is it working with word wrap? (assuming there is support for it, I know by experience the features might overlap, because visible lines and actual lines are no longer the same thing)
I'd also suggest it could have some language awareness (GDScript, JSON, GLSL, C#) since indentation is not always part of the language syntax (even in GDScript as already noted for dictionaries and arrays). It doesnt need to be "language" though, maybe just some patterns that should produce folding.

@DriNeo
Copy link

DriNeo commented Nov 16, 2017

Just a cosmetic suggestion. The arrows are big in my taste. Tiny triangles, like in the scene dock, maybe better.
Anyway this PR is a very nice improvement. Sorry if my english is rough.

@blurymind
Copy link

blurymind commented Nov 16, 2017 via email

@volzhs
Copy link
Contributor

volzhs commented Nov 16, 2017

@blurymind yeap. @djrm the professional will take care of it, I think.

void TextEdit::set_draw_fold_gutter(bool p_draw) {
draw_fold_gutter = p_draw;
if (draw_fold_gutter)
set_hiding_enabled(true);
Copy link
Member

Choose a reason for hiding this comment

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

Missed this to disconnect gutter from folding :D

@kitbdev
Copy link
Contributor Author

kitbdev commented Nov 16, 2017

Fixes for smooth scrolling, last line folded, and others. Cursor now shape changes over icons.
Scrolling now ranges from 0 to the total number of unhidden lines, so it does not match cursor.line_ofs anymore.

@Paulb23
Copy link
Member

Paulb23 commented Nov 16, 2017

Great work, scrolling over the hidden lines feel a lot better!

I noticed a few problems when "Scroll Past End Of File" is disabled and "Smooth Scrolling" is enabled.

  • When there are a lot of folded lines it's impossible to scroll to the last line.
  • It also starts 2 lines from the end if the content is less then a screen full.
  • You can also scroll past the first line when the content is less then a screen full.

@kitbdev kitbdev force-pushed the code_folding branch 2 times, most recently from 64a0109 to 18df45d Compare November 18, 2017 02:53
@kitbdev
Copy link
Contributor Author

kitbdev commented Nov 18, 2017

Fixed the Scroll past end of file bugs. Took me longer than I would have liked, but scrolling should work now.
I dimmed the icons a bit and I took the GUITabMenu icon and transposed it so it now looks like ellipses for the end of line icon :)
image

@remorse107
Copy link
Contributor

I am a fan of the new look. Really looks like it belongs now. I hope this makes it into beta.

@Zylann
Copy link
Contributor

Zylann commented Nov 18, 2017

I think the folding icons are still too wide...
Also, there should be a space between the end of the line and the ellipsis icon.

@djrm
Copy link
Contributor

djrm commented Nov 18, 2017

there are two arrow icons that are smaller, and would be better in this case
GuiTreeArrowDown and GuiTreeArrowRight, that would make it look better. also i think i will have to create an ellipsis icon

@kitbdev
Copy link
Contributor Author

kitbdev commented Nov 19, 2017

Updated to use the smaller icons. Thanks!

bool TextEdit::is_folded(int p_line) const {

ERR_FAIL_INDEX_V(p_line, text.size(), false);
if (p_line + 1 >= text.size() - 1)
Copy link
Member

Choose a reason for hiding this comment

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

This line should be p_line + 1 >= text.size() else you can't unfold the last line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The last line isn't able to be folded. I determine whether a line is folded by seeing if this line isn't hidden and the next one is hidden, so the next line needs to be less than or equal to text.size() - 1 for that check.

@Paulb23
Copy link
Member

Paulb23 commented Nov 19, 2017

There are few more scrolling bugs:

  • With "Smooth Scrolling" disabled everything if offset by one, (ie clicking line 2 will select line 3)
  • With "Smooth Scrolling" enabled and scrolled to the top, clicking any line will move the scroll down a line.
  • With "Scroll Past End Of File" disabled scrolling to the bottom of the file and hiding lines, will not update the scroll/viewport.
  • "Smooth Scrolling" enabled does not draw enough lines resulting in a small gap at the bottom.

@HummusSamurai
Copy link
Contributor

I vote to have this merged for the feature freeze asap and the remaining trivial bugs closed later.

@blurymind
Copy link

Yes please :)
Perhaps if it gets merged we will be able to find possible bugs faster too

@akien-mga
Copy link
Member

Alright, let's merge :) Please open issues for the known issues mentioned above which haven't been addressed yet.

@akien-mga akien-mga merged commit d826b04 into godotengine:master Nov 19, 2017
@kitbdev kitbdev deleted the code_folding branch February 13, 2018 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.