Bug 314395 - Automatic indentation of subsequent lines in kate is erratic
Summary: Automatic indentation of subsequent lines in kate is erratic
Status: RESOLVED FIXED
Alias: None
Product: frameworks-ktexteditor
Classification: Frameworks and Libraries
Component: indentation (show other bugs)
Version: unspecified
Platform: Ubuntu Linux
: NOR normal
Target Milestone: ---
Assignee: KWrite Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-04 02:30 UTC by sparhawk
Modified: 2019-07-08 20:59 UTC (History)
7 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.61.0


Attachments
Patch which should work to fix this bug (or change this feature) (1.30 KB, patch)
2013-04-02 00:58 UTC, Luke San Antonio
Details
A new patch which introduces a new option and modeline (10.34 KB, patch)
2013-04-08 00:17 UTC, Luke San Antonio
Details

Note You need to log in before you can comment on or make changes to this bug.
Description sparhawk 2013-02-04 02:30:05 UTC
When I use kate, it automatically indents the next line to the same level of the current line. Hence, if the current line begins with x tabs, after pressing enter, the new line starts with x tabs. However, this only works consistently if x ≥ 1. If the current line begins with no tab, then one of two things happens. If there are other character(s) on the current line, then the next line also start with no tab (as expected). However, if the current line is blank, then the next line starts with the most-recent non-blank line's indentation. This seems counter-intuitive to me.

If important, I have kate's "Default indentation mode" set to "Normal".

Reproducible: Always

Steps to Reproduce:
1. Start on indented line.
2. Press enter, and see same indentation on next line.
3. Delete indentation, so that cursor is at the beginning of the line.
4. Press enter again.
Actual Results:  
Next line is indented as per two lines above that.

Expected Results:  
Same indentation as line above that (i.e. none).

I'm using Kate 3.9.4 (which was not one of the options above). This is latest 12.10 from the Ubuntu repos.
Comment 1 Luke San Antonio 2013-04-01 22:39:28 UTC
Hmm, looking at part/utils/kateautoindent.cpp it seems like this completely intended. Therefore a patch (which would be trivial) most likely won't be accepted.

I guess the point is: who's really to say what the "Normal" role of an indentation mode should do.

That being said, I agree with you entirely since I think a normal indentation mode should repeat the indentation level of the previous line, the "Normal" indentation mode in Kate gives less decision to the writer and more power to editor because it thinks it's helping.

I'll add a patch in a bit, basically for you and me, I doubt it will be accepted, because as I said, it is definitely indented. But what the hell - Luke
Comment 2 sparhawk 2013-04-01 23:38:38 UTC
I did have some suspicions about this being a "feature", as I thought it such an obvious "bug" that it should have been fixed a while ago. I still can't really see how this could be expected behaviour, though.

Thanks for the offer of writing a patch! However, if you are unsure whether it'd be accepted, instead of modifying the behaviour of "Normal", you could create a new option instead? This might be more likely to be accepted.

Having said that, perhaps some of the devs could chime in and explain if the default behaviour of "Normal" is really expected behaviour, or really a bug, and should thus be fixed.

Cheers.
Comment 3 Luke San Antonio 2013-04-02 00:58:41 UTC
Created attachment 78566 [details]
Patch which should work to fix this bug (or change this feature)

Here's a patch I wrote, it basically forces the program to use the previous line (if there is one) to indicate indentation if the user is currently in "Normal" Indentation Mode. This kind of bothers me though, since changing this kind of behavior can be explosive, programmers get used to things and after that it's hard to change back, I can't imagine this being accepted with quiet dignity and grace.

I guess one possibility would be to add an option to 'Configure Kate... Maybe in Editor Component -> Editing -> Indentation -> Indentation Properties. I will take a stab at it but I have very little experience with Qt.

Don't get me wrong, I agree that a new indentation mode might be less reluctantly accepted, but to change one little feature, it seems sketchy to me... I mean, what would we even call it so it won't be unclear? =D

For now my patch should fix your problem, but I think more work is necessary.
- Luke
Comment 4 sparhawk 2013-04-02 03:57:33 UTC
Great! I'm unfamiliar with applying patches though. Could you please point me to an appropriate resource (or tell me how if it's quick and easy)?

You are probably right with all your other comments. An additional menu item would probably be best, but possibly complicated to code. But as I mention before, I can't imagine how this could be a feature, so (unless we hear otherwise) my feeling is to squash the bug!
Comment 5 sparhawk 2013-04-02 10:55:41 UTC
Ah don't worry about applying patches. I worked it out myself.

However, this patch doesn't seem to work for me. Now under "Normal", I don't get any automatic indentation. That is, if I press return at the end of an indented line, there is no automatic indentation on the next line.
Comment 6 Luke San Antonio 2013-04-02 22:52:10 UTC
I think I had this problem too, It was strange and I will look into it, but I also remember it worked after that once I reopened to application. I am going to try it again right now though. So I'll get back to you.
Comment 7 Luke San Antonio 2013-04-02 22:53:04 UTC
*but I also remember it worked after I reopened the application once

That was a bad couple of typos, sorry.
Comment 8 Luke San Antonio 2013-04-02 22:56:33 UTC
Strange, I have the same issue and one crash even, I was testing it yesterday, and I know it worked then... aw well.

I guess you should just disregard this patch till I check it out...

Sorry about that, it seemed to be working before... I'll keep working on it - Luke
Comment 9 sparhawk 2013-04-02 23:30:54 UTC
Actually, I can get the previous functionality back if I reset the indentation mode. i.e. Settings > Configure Kate > Editing > Indentation > Default indentation mode: select something else, click apply, then select Normal again, then apply.

However, the behaviour now is just like before. i.e. the normal automatic indentation works, but the bug still exists as before.
Comment 10 Luke San Antonio 2013-04-03 00:48:27 UTC
Your right, the original behavior appears in the patched version as well. I guess the patch is meaningless... It seems weird though, because I could have sworn it was working a couple nights ago, but then again I did do most of that coding at three in the morning, so you never know...

I'm checking it out, so... I guess I'll report back...

Also I do think a dev in here would help a lot, I'd like to see one of their takes/opinions on it.

 - Luke
Comment 11 Luke San Antonio 2013-04-03 02:54:16 UTC
Okay, I think I have an explanation

When I originally attempted to write a patch for Kate, I had a little problem, the code I was adding was not being executed. This made no sense to me, I even outputted some text to the terminal. Nothing!

Why? Because KDE puts most of it's application's functionality in shared libraries. This is all well and good, however, when I ran the compiled Kate program it used the shared code located somewhere deep in my system. This is a problem, because now my *new* application code will not be executed. So the good people at KDE provided us with a script. This script sets the program up to use the *newly, compiled code!* It's a simple shell script. But you have to know that it is there.

This is why running the Kate program did not execute my code. Because it was using the wrong libraries, *I needed that shell script!*

Finally I figured it out... In the same directory there was another executable called 'kate.shell'. Running this solved all my problems. My new code was executed.

Now I, when first starting off writing this patch had no idea kate.shell existed and since I made this mistake I can only assume others can and will as well. Therefore I am going to give you a step by step guide which I did to make this patch, the same patch, work as expected.

1. Install git, cmake, and a compiler. I assume you have all these since you said you compiled with the patch already.

2. Clone the Kate repository: Execute `git clone git://anongit.kde.org/kate` in your terminal.

3. `cd` into the newly created directory

4. Download that same patch and type `git am <path-to-patch>` This applies my patch to the code.

5. `mkdir build && cd build` Make a new directory to build in (and go into it) (I don't usually support in-source build, but they suggest it on Kate->Get it, so what the hell)

6. Configure the project (in the build directory): `cmake .. -DBUILD_KTEXTEDITOR=1 -DCMAKE_BUILD_TYPE=Debug -DCMAKE_INSTALL_PREFIX=~/kde/usr`

7. Run `make`

8. At this point you should be able to test your built Kate by running: `./kate/app/kate.shell`

9. **Very Important** It seems Kate opens up with no indentation even though it says it is on "Normal." This is probably a bug, but it is not my bug, I did a little experimentation and found that this occurs with or without my patch... Therefore just go into Tools - > Indentation - > Normal.... click it once....

And Now

Try going through with your "Steps to Reproduce"... And hopefully, you should get your expected result.

10. If you are happy with your build run `make install` (No, you don't root privileges)

11. Make the script mentioned on http://kate-editor.org/get-it/

Yeah, I really hope this helps you out.

Or as you would say:
Cheers.
Comment 12 Luke San Antonio 2013-04-03 03:05:20 UTC
I forgot to mention (if it wasn't implied):

**You must always run this new, compiled Kate with the run.sh script provided on http://kate-editor.org/get-it/ or the system will load the wrong libraries and you will not get the patch.*

What I do is set up the Kate application in the little pop up menu on the lower left corner of my screen to use the script to launch and it works like a charm...

Once again:
Cheers.
Comment 13 sparhawk 2013-04-03 04:29:14 UTC
Wow, thanks for the in-depth explanation. I'll test it out tonight. FWIW, I actually had a totally different way of applying the patch. I think my method replaced the system version of kate, which is think is better. (The package manager allows reversion to the official version.) This also means that I can leave all of the document associations and the launcher shortcuts, etc. I'm using Kubuntu, so I did

sudo apt-get build-dep kate
apt-get source kate
cd kate-4.9.5/
patch -p1 < ../patch
# edit debian/changelog to create additional entry with new version number (i.e. *.patched).
debuild -us -uc -i -I
# I might have needed to `dpkg-source --commit`, but I forget.
sudo dpkg -i kate_4.9.5-0ubuntu0.1.patched_amd64.deb kate-data_4.9.5-0ubuntu0.1.patched_all.deb katepart_4.9.5-0ubuntu0.1.patched_amd64.deb
Comment 14 Luke San Antonio 2013-04-03 21:58:26 UTC
Okay, I'd like to see if you get a different result. The method you did probably would work on most projects, but as I have concluded, Kate and other KDE projects seem to have a problem with using the correct code at runtime. That's why they made that script that I mentioned. Also the method I described is similar to the 'Get it' page on the Kate website. So I assume it's more reliable.

Yeah, I'm totally looking forward to your result, it should work, but - ya know - no one can ever tell...

 - Luke
Comment 15 sparhawk 2013-04-06 01:26:19 UTC
I tested with you installation instructions (up to step 9), and that patch works perfectly!

To be honest, I'm not really happy with the workaround of having two kates, though. As I mentioned, I suspect this will break some of the application associations with xdg-open and dolphin, etc., and so I'd much prefer to patch the existing kate. Of course, the best solution would be to permanently fix the bug in trunk, but I suppose we'll need to wait for devs to reply to this report. Thank you for the fix, though.

I noticed one other small issue. When selecting indentation mode, I only saw "none" and "normal" as options. Using vanilla kate, there are a bunch of other options, such as "python", "ruby", etc.
Comment 16 Luke San Antonio 2013-04-06 04:18:59 UTC
I'm glad it worked! Totally happy.

So a couple things I want to mention:
1. The indentation mode thing happened to me too, if you were to install it however, they come back
     - I assume this is because when Kate is run from the build directory they aren't found, but when installed, they are put in the appropriate locations so that they can be found...

Also:

2. In my current setup, whenever Kate is needed to open files, my patched version will run. This is actually a great alternative to having two parallel Kate editors on your system because the system Kate is still there and updated, but your system, and KDE will load up files in your new Kate. I'll tell you how to do this until we get some dev feedback:
  - I assume you have that Task Manager(?) bar on the bottom of your screen, or the left, or the right... Anyway go the the KDE logo, aka the "Kickoff Application Launcher" and click it. Now right click where it shows your username, click "Edit Applications"
  - This is where xdg-open, dolphin and KDE look to find the applications to run.
  - Find Kate and change it from something like "kate -b %u" to something like "$HOME/kde/run.sh kate -b %u"
     - Basically append the path of the script you copied from the "Get it" page on the Kate website to the current value of the command field.
  - There is a lot more detail on install Kate under the "Get it" section of the Kate website, so check that out.
  - I have tested it, and now, xdg-open, dolphin, and any KDE menus load up to my patched Kate.
  - That's it.
So glad the patch worked though, and I totally understand if you don't want to install it on your system, I just thought you might consider it if you knew most things, if not everything, will adapt to the new Kate.

Hope this helps, and if you don't get anything, just ask, I'll get the response - I have this page bookmarked! =D

 - Luke
Comment 17 Luke San Antonio 2013-04-06 04:21:51 UTC
Sorry the fourth bullet down from my second point is:

There is a lot more detail on install Kate under the "Get it" section of the Kate website, so check that out.

That should be:

There is a lot more detail on install*ing* Kate under the "Get it" section of the Kate website, so check that out.

I'm sure you could have figured that out, but I am a nerd about typos. =D

See ya - Luke
Comment 18 sparhawk 2013-04-06 04:29:48 UTC
Thanks very much for the advice. This certainly does seem like a good workaround. Hopefully we'll see some developer action in here soon. I'll wait a week or so, and if no one replies, then I'll try the rest of your advice. It certainly seems like it'd fix the problems I mentioned. I guess the other thing with this solution is that we'd have to constantly patch new repo versions, so again, if devs are happy to patch trunk then this would be preferred.

Cheers and thanks again.
Comment 19 Christoph Cullmann 2013-04-06 14:50:36 UTC
Actually, you are right, this behaviour is a feature. The idea behind is, that you often write stuff like:

   int x;

   x = 0;

and sometimes you have empty lines in between like here.
Now if you press return in the empty line, normally you want that you get 2 spaces indentation for the next.

That is counter-intuitive if you want to really go back to zero indentation but make there a empty line first. Not sure if we should change the current default behavior.
Comment 20 sparhawk 2013-04-06 21:45:40 UTC
@Christoph: When I'm coding, in this kind of situation, I'd be happy to have the interstitial "blank" line contain a tab. If the indentation mode reflected our discussion above (i.e. reverted to zero tabs) then it'd make more sense in both situations, with the presumption that people were happy to have "blank" lines contain tabs as above. At least in this case, the reduced functionality results in trailing tabs rather than leading tabs (and IMO is less irritating).

If you disagree with this, at the very least, do you think there is an argument for introducing an additional option to kate to allow this behaviour? Or perhaps an alternative "Normal" indentation?
Comment 21 Luke San Antonio 2013-04-07 02:03:32 UTC
I guess my idea would be introduce an option with the default being the current behavior, that way regular users can use Kate without noticing a difference while the informed can switch it off.

I guess my point to adding an option is: no indentation is just as significant as two spaces or four spaces of indentation. Also many other editors indent this way (Gedit, for one) so it's usually a good idea to get some consistency, or at least provide an option for it.

I was thinking we could put this option in Editing Component -> Editing -> Indentation -> Indentation Properties... Maybe =D

- Luke
Comment 22 Dominik Haumann 2013-04-07 10:00:51 UTC
I'm against adding an option. I consider this a feature, and in fact, if you want exactly the same indentation like the previous line, just use <Shift>+<Enter> (= smart return). Smart return copys all non-letter characters of the previous line.

Another option: It is trivial to write an own indenter in JavaScript, that's after all why we have the scripting support: http://docs.kde.org/stable/en/applications/kate/advanced-editing-tools-scripting.html
Comment 23 sparhawk 2013-04-07 11:03:39 UTC
Thanks Dominik, those are both good suggestions. I'm not really sure how many users feel that my complaint is the preferred behaviour, but I guess two of two devs so far is a decent hit rate. :) Obviously, I feel that it'd be great as a stock option, but I'll defer to this majority.

I think smart return does exactly what we want, although I can imagine this keystroke will take a bit of getting used to.

Thus, I guess it'd be great to write the custom indenter. I assume that custom indenters would just be selectable from the normal kate indentation settings, and hence unchanged over kate upgrades? I'm not familiar in any way with javascript, but I can script in other languages, and I assume it'd pretty easy to modify the default "Normal" (and the documentation seems thorough). The linked page states that "a default KDE installation ships Kate with several indenters. The corresponding JavaScript source code can be found in $KDEDIR/share/apps/katepart/script/indentation." However, I couldn't find ~/.kde/share/apps/katepart/script . Am I looking in the right place?
Comment 24 Luke San Antonio 2013-04-07 14:44:04 UTC
Yeah, I guess a JavaScript indenter is the way to go. Just because I agree with sparhawk in that smart return will take a bit getting used to.

Also, @sparhawk, those scripts you are looking for are system wide. I found them at /usr/share/kde4/share/apps/katepart/script

Although, out of curiosity, why not an option which could have the default setting of the current behavior so that there aren't two indentation modes like "Normal" and "Normal - Modified" etc? Now I'm just wondering.

 - Luke
Comment 25 Luke San Antonio 2013-04-07 20:13:03 UTC
I guess my opinion on this whole option-or-no-option thing is: Why not add the choice, why not give more power to user, how could the project go wrong by adding more possibility?

That's why I feel like an option and patch would be a great idea!

I want to hear what all of you feel about it though.

 - Luke
Comment 26 sparhawk 2013-04-07 21:47:29 UTC
On my system (Kubuntu), there are some files that potentially look like the indentation files at  /usr/share/kde4/apps/katepart/script (I tried to search for "indentation", but I guess the reference to this was not as a filename, but generic placeholder). However, I couldn't find anything called "Normal", so I guess this is not coded in this format. I suppose I should have known this, as Luke already hacked into this code earlier. (I'd expect it'd be easier to base our modified script of "Normal", was all.)

As for introducing an option, I'd certainly vote for this, since this behaviour makes the most sense to me. (It makes so much more sense that the classification of the default behaviour as a "feature" was not obvious at all.) In fact, thinking about it more, the case as stated by Christoph doesn't make a lot of sense to me, when scaled. If you wanted interstitial lines to have no indentation, and revert to the original indentation, then what about having multiple interstitial lines? e.g.

      int x;



      x = 0;

In this case, you would have to delete the indentation *every single time* you create a new interstitial line. It makes more sense to me to delete it the first time, then have kate follow this indentation, then for the user to manually re-create the indentation. Even for this use case, you'd save keystrokes by kate following this behaviour.
Comment 27 Luke San Antonio 2013-04-08 00:17:18 UTC
Created attachment 78719 [details]
A new patch which introduces a new option and modeline

Alright, So I wrote the patch which should do the job, I know you all said that it might not be the best idea, but I wanted to show you a (working) proof of concept of what I was thinking before you made the final judgment.

I guess the important thing of this patch is, by default, the current behavior is preserved, so applying it could be done silently until something feels the need to change it. I also added a modeline, which I am pretty sure works just as well as any other, but because I have no experience with Kate development or KDE development for that matter, I would appreciate some critique.

So I hope you all consider this patch =D
 - Luke
Comment 28 St Weiss 2013-10-18 21:50:15 UTC
Has this patch been considered by the developers yet? Kate is a sweet editor; the unusual indentation behavior is the only thing bothering me about it. I would very much appreciate an option to change the default.
Comment 29 sparhawk 2013-10-18 23:16:33 UTC
FWIW I've been using `zim` recently, and its default behaviour is exactly as I expected kate to do (i.e. as stated in the OP).
Comment 30 spaceman 2013-10-19 09:36:30 UTC
*** This bug has been confirmed by popular vote. ***
Comment 31 Christoph Cullmann 2019-07-08 20:59:55 UTC
Git commit 01526bfcb7e811dbc787f2fdf2924f30d7f8324c by Christoph Cullmann, on behalf of Ahmad Samir.
Committed on 08/07/2019 at 20:59.
Pushed by cullmann into branch 'master'.

Add an action to insert a non-indented newline

Summary:
Currently pressing Enter to insert a new line, KTextEditor will,
in most cases, indent the new line with the same indentation as
the previous line, which makes sense when writing code. But there
is a viable use case where a user may want to insert a new line to
separate blocks of code that have different context/functionality
inside a function, in that case that line should be non-indented,
having trailing spaces on an empty line isn't good, IMHO.

The new action can be triggered with Ctrl+Enter. And the current
default behaviour is preserved.
FIXED-IN: 5.61.0

Test Plan:
- Open a file with some C++ code, put the cursor at the end of an indented
  line, and press Enter to insert a new line
- Note that the new line is indented
- Repeat the first step, but press Ctrl+Enter; the newly inserted line has no indentation

Reviewers: #ktexteditor, cullmann, dhaumann

Reviewed By: #ktexteditor, cullmann

Subscribers: bruns, mickaelbo, kde-frameworks-devel, kwrite-devel

Tags: #kate, #frameworks

Differential Revision: https://phabricator.kde.org/D22276

M  +6    -3    src/document/katedocument.cpp
M  +3    -1    src/document/katedocument.h
M  +17   -0    src/view/kateview.cpp
M  +8    -0    src/view/kateview.h

https://commits.kde.org/ktexteditor/01526bfcb7e811dbc787f2fdf2924f30d7f8324c