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

Allow unindent without selection #60904

Merged
merged 1 commit into from
Jan 18, 2023
Merged

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented May 9, 2022

Revival of #50289
Fixes #50279
Fixes #68154

@KoBeWi KoBeWi added this to the 4.0 milestone May 9, 2022
@KoBeWi KoBeWi requested a review from a team as a code owner May 9, 2022 15:53
@KoBeWi
Copy link
Member Author

KoBeWi commented May 10, 2022

This seems to fail 2 test cases.

test[tab]

is expected to become

test

after unindent (i.e. whitespace to the right is removed). This behavior is probably what caused the original issue. Any reason why it would work like that? Is it common in code editors?

@KoBeWi
Copy link
Member Author

KoBeWi commented Oct 29, 2022

Rebased. I also changed the failing test. tbh the two behaviors aren't really conflicting (i.e. Shift+Tab can remove trailing tab and unindent if there isn't one), but I'm just reviving a PR here; it's not like I know what it does >_>

@Paulb23 Review would be appreciated, the issue gets in my way too often.

@MewPurPur
Copy link
Contributor

MewPurPur commented Dec 2, 2022

Makes sense to me. I don't see why the current implementation would be desired - if I need to remove whitespace, I can just press Backspace. Idk about removing previous whitespace, instead of just tabs at the beginning of a line, but it seems pointless to me.

scene/gui/code_edit.cpp Outdated Show resolved Hide resolved
doc/classes/CodeEdit.xml Outdated Show resolved Hide resolved
@KoBeWi
Copy link
Member Author

KoBeWi commented Jan 12, 2023

I reworked the PR. It no longer removes do_unindent(), it simply changes the method used for dedent action.

@Paulb23
Copy link
Member

Paulb23 commented Jan 14, 2023

I would continue to remove do_unindent then I think we can merge this for 4.0, as this would fix #68154. Otherwise it requires a bit of a rework for multi-carets. Can add it back later if needed.

@akien-mga
Copy link
Member

What's the difference between the remaining do_indent and indent_lines?

@Paulb23
Copy link
Member

Paulb23 commented Jan 15, 2023

Some of those test are probably still worth keeping.

What's the difference between the remaining do_indent and indent_lines?

do_indent is called when the user presses tab, so will insert a tab/space at the caret(s). However, if there is a selection it will call indent_lines, which will move / indent lines that have a caret or selection. Similar to how the JetBrains IDE's work.

@KoBeWi
Copy link
Member Author

KoBeWi commented Jan 15, 2023

Some of those test are probably still worth keeping.

So should I change them to unindent_lines()? Which ones exactly?

@Paulb23
Copy link
Member

Paulb23 commented Jan 16, 2023

So should I change them to unindent_lines()? Which ones exactly?

Yeah I think most of them should be okay apart from the ones under the /* Should indent inplace. */ heading.

@akien-mga akien-mga merged commit f40f1e0 into godotengine:master Jan 18, 2023
@KoBeWi KoBeWi deleted the 👻👻👻 branch January 18, 2023 22:05
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants