godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.13k stars 21.17k forks source link

The code editor deletes more than expected when using CTRL + DELETE #76093

Open twoco opened 1 year ago

twoco commented 1 year ago

Godot version

v4.0.2.stable.official [7a0977ce2]

System information

Windows 11, Forward+, RTX 2070 Super

Issue description

When using CTRL + DELETE, the code editor deletes more than expected. In this case of deleting the indent (to fix it), the first word is deleted, instead of just the spaces/tabs.

In Godot 3 it works as expected. This issue exists since Godot 4. It's a regression.

Steps to reproduce

  1. Open a script (GDScript) in Godot 4.
  2. Move the cursor in front of a line in the indent section.
  3. Press CTRL + DELETE and see that the code is removed. Expected: Only indent (spaces) should be deleted.
  4. Do the same in Godot 3 and see that it works as expected: Only delete the spaces not the code.
Godot 3 Godot 4
godot_3 godot_4

Minimal reproduction project

No project needed. Just test this is any code editor.

Additional information

If the cursor is at the end of the line but before trailing spaces of the line, the lines are not deleted with CTRL + DELETE. It's maybe because this function only work on words (not space).

MewPurPur commented 1 year ago

I'm 50/50 on whether this is a bug. Currently Ctrl + BS/Del just goes one space back/forward and deletes everything up to before/after the last/next word. It seems useful to have an exception for many whitespaces so you can neatly delete them all with one stroke, so while I don't think it's a bug, I'm in favor overall.

twoco commented 1 year ago

It's unexpected and unusual behavior. The term "bug" does not always have to refer to an error. It also includes user experience and unexpected behavior. In that case I would definitely say it's a bug. Intentionally implemented or not. But I don't want to argue about semantics... ^^

I mean it works in all editors I know including VS Code and Godot 3. CTRL delete/bs should handle space blocks. Just like in Godot 3 and all other editors even Windows notepad :D

VS Code

MewPurPur commented 1 year ago

I'm not arguing semantics, just explaining what causes the current behavior and trying to understand why you find it unexpected.

But anyway, VSCode seems like the gold standard for our script editor team so I think this is justification enough.

twoco commented 1 year ago

It was meant in general. :) The current behavior is unusual and therefore unexpected. It's annoying when words are deleted from this cursor position. Especially when developing in such a python-like language where indentation is important and where no auto formatter is available. Code formatting and refactoring is much more comfortable with the expected behavior known from Godot 3, VS Code and 99% of the rest of editors. So you can e.g. paste code and format without hassle. Thanks for fixing it. 👍 Btw. this is for all editors in Godot 4. Incl. the editor for shaders. I wish there was an option to auto format...

twoco commented 1 year ago

godot4_shader_editor Expected cursor stops before //. This works better in Godot 3. Is this an intentional change in Godot 4? Under accidentally? I would revert it. Maybe your changes already fix this. I'm just posting this for completeness.

We can see, it's not only about delete / backspace. It's about word select. E.g. with SHIFT + CTRL and arrows. It's no fun navigating Godot 4 with this change.

MewPurPur commented 1 year ago

@twoco Have you tested my PR?

We can't "just revert it", TextEdit is different from what it was in Godot 3 (i.e. having to account for multiple carets makes _delete() ten times as complicated). Stuff like this had to be rewritten and behaviors like these are oversights from that work.

We can see, it's not only about delete / backspace. It's about word select. E.g. with SHIFT + CTRL and arrows. It's no fun navigating Godot 4 with this change.

Is there an issue for this?

twoco commented 1 year ago

Have you tested my PR?

I tested the CI artifacts with the following result.

godot4_745

CTRL + DELETE looks good. But CTRL + ARROWS not yet. I just thought it's the same code behind the CTRL key function (word jump/select/delete).

godot4_745_2

The CTRL + BS seems to still have the problem. It deltes to much. Should stop on content (not spaces). Maybe we can manage the code to handle CTRL navigating (arrows) and select (shift) and delete (delete/bs). I don't know. I would help to code. I'm not a cpp dev and have not exp how to write an editor. So complaining is all I can do for now. ^^ But my idea is to handle the behavior when Ctrl key is pressed (arrows, delete/bs key) and select the content with Shift. For delete/bs I just would call the ctrl method (which is pressed anyway), read out what would be selected (shift) and delete the selection. ... But idk, maybe you have better ideas. :)

Ok the title of this issue is about "delete". But in core it's about CTRL key behavior.

twoco commented 1 year ago

Ok, forget what I wrote. More or less. I tested the CTRL in VS Code again. And it seems the behavior with arrow keys works in the same weird way as Godot 4 currently do. Not my prefered behavior but ok.

vscode_again (VS Code)

This means the only thing I would add to the ToDo fixes list is the CTRL + BS behavior. In Godot 4 it should only delete the space block. Not the content ") at the end of the line. Sorry for confusing.

And this is the browser's behavior (win 11, chrome, github)

browser_chrome_win_github

func _input(event):
  if event is InputEventKey and event.pressed:
    if event.keycode == KEY_T:
            print("T was pressed")                

Screw it. 😄 But ok, we could do things better in Godot. So finally, the current solution of your PR looks good (CTRL + DELETE). The backspace behavior seems to be very random in editors. Thanks for the Ctrl-Delete fix 👍

Let me know, what do you expect when using CTRL with Arrows/Shift/Delete/BS? Stop between space and word block or like the current behavior. I thinks it feels better stop cursor between space and word (content / not space) block. But anyway...

twoco commented 1 year ago

Do you want A or B?

solutions

Not sure, but B seems the common behavior in most editors. And A for delete/bs. Personally I would just behave like A for all cases. It's safer and more comfortable. Pressing Shift just selects it. And with delete/bs it can be deleted by select and remove selected content. Anyway.. Your PR seems to work for CTRL + DELETE and would close this issue. I would fix the CTRL + BS from end of line. But only if you want... And sorry for the amount of text/content here. ^^ We could also just copy VS Code behavior or embed it in Godot. ;)

MewPurPur commented 1 year ago

I would expect Ctrl+Left to stop at the same places that Ctrl+BS does, likewise for Ctrl+Right and Ctrl+Del. Maybe you or someone else who sees this can use my PR for inspiration. Working on the the script editor is exhausting and I'm taking a break from it :sweat:

Deozaan commented 1 year ago

Do you want A or B?

A seems much more intuitive and useful to me.

twoco commented 1 year ago

@MewPurPur NP I tested the editor build of your PR on windows (Artifacts) and it works great. At least it fixes the issue of case CTRL + DELETE (this issue). Good job. Sorry if I confused with my text (or bad english). There are just some open points to fix in the editor what work better in e.g. Godot 3 and VS Code. But I would say we could merge the PR if the code reviewer gives the ok and close this issue. We can just create a new issue for the other points. Thanks for your changes. I'm sure all developers who are facing this problem in current Godot 4 version can't wait for this fix to be released. So it would be great if we cherry-pick this for a upcoming patch version (4.0.3).

Update: I've tested again and it seems that we only need to fix the CTRL + BACKSPACE issue to meet the same or similar behavior as in VS Code. Because CTRL and DELETE / BS handles space blocks different as CTRL with ARROWS. I would also prefer when arrow keys acting the same way (both same behavior), but I would maybe create a new issue for that.

Godot 4 PR build VS Code
godot_73580 vscode
This PR fixes CTRL + DELETE which is awesome. CTRL + BS not yet fixed. We could also fix CTRL + BS in same way to handle spaces. Like in VS Code.

What do others think about it (@Calinou)? I'm fine with the current fix in PR #73580. But the CTRL + BS would complete the fix this issue. For further concerns like CTRL + Arrows etc, we can create another issue. I really can't wait to have this fix. :) So I would close this with the current PR. In best case in upcoming 4.0.3.

damil42 commented 1 year ago

This is really annoying and worked much better in Godot 3. I also wish there was an automatic formatter for e.g. indentation. Anywaay, the spaces should be handled as a word/block like in other editors e.g. VSCode and Godot 3. Looking forward to the fix.

MewPurPur commented 1 year ago

I'm not maintaining the fix, but it's 90% done. Please supersede me if you can write tests.

domske commented 1 month ago

That is one thing I miss from Godot 3. Especially when trying to correct the indentation alignment. Which is one basic in that python-like language. I fall into the trap every time. I don't know the background or reasons behind, but it would be great if we could fix it. I know, it's just another comment. I wish I could help. But I'm not experienced enough in CPP and this project source code. It would be better if this were done by someone who knew the subject better. It's maybe just a tiny change to handle the space blocks (not meant presumptuously). What has been changed here from Godot 3 to 4? It's in scene/gui/code_edit.cpp, right? Highest props for whoever solves this.