Bug 317299 - Rename assistant (for local variable) broke indents when we using Tabs as indent
Summary: Rename assistant (for local variable) broke indents when we using Tabs as indent
Status: CONFIRMED
Alias: None
Product: kdevelop
Classification: Applications
Component: general (show other bugs)
Version: git master
Platform: Mageia RPMs Linux
: NOR normal
Target Milestone: ---
Assignee: kdevelop-bugs-null
URL:
Keywords: usability
: 426535 (view as bug list)
Depends on:
Blocks:
 
Reported: 2013-03-24 21:56 UTC by Piotr Mierzwinski
Modified: 2020-09-21 19:59 UTC (History)
5 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Piotr Mierzwinski 2013-03-24 21:56:27 UTC
Use rename assistant to rename local variable in function made inserting string of spaces before name (used below in code).


Reproducible: Always

Steps to Reproduce:
1. Example of code:
	QString str;
	if (true)
		str = "abc";

2. Put '2' or something else on end of str declaration. Example like this: "str2"
3. "Rename assistant" ask you question: "1 - Rename "str" to "str2" | 0 - Hide"
4. Chose "1"
5. str has been renaming but before str = "abc" appears string of spaces
Actual Results:  
	QString str2;
	if (m_error)
		      str2 = "abc";

Expected Results:  
	QString str2;
	if (true)
		str2 = "abc";
Comment 1 Piotr Mierzwinski 2013-05-24 19:15:37 UTC
Please pay attention that in "Example of code" (point 1) has been used tab's indention (not spaces). This is a necessary condition when bug will appears.
It seems, that Renamer tool doesn't respect type of indention and always try to use spaces. It's very annoying for someone who uses tab to make indents.
Comment 2 Piotr Mierzwinski 2015-11-07 18:24:12 UTC
This is also happen in KDevelop 5 (coming from master branch).
I have set tabulators as indention in KDevelop configuration.
Comment 3 Piotr Mierzwinski 2015-11-10 21:11:30 UTC
When I rename from longer to shorter name (removing couple of leading characters) then removed characters are replaced with spaces. What causes breaking indention.
Tested after fix Bug 354971
Comment 4 Igor Kushnir 2020-03-21 12:14:50 UTC
I can reproduce this bug with some formatter styles, but don't think it's easy to fix. KDevelop intentionally applies selected style reformatting when the user refactors the code, e.g. renames a variable.

The relevant KDevelop code actually has a very poor performance: it reformats the whole file for each use of the variable being renamed just to "fix according to the selected style" the whitespaces around the variable names it changes. I hope this code will be thrown out in favor of a better clang-format implementation in the future.

As a workaround just pick a style where the spaces don't get inserted in your code. The Qt Artistic Style works for me.
Comment 5 Piotr Mierzwinski 2020-03-24 21:40:59 UTC
(In reply to Igor Kushnir from comment #4)
> the selected style" the whitespaces around the variable
> names it changes. I hope this code will be thrown out in favor of a better
> clang-format implementation in the future.

I wish this be implemented as well, but I'm not sure if this is one of point in KDevelop developers.
Notice please that development of KDevelop recently is going really slowly.
Comment 6 Igor Kushnir 2020-09-15 02:55:50 UTC
*** Bug 426535 has been marked as a duplicate of this bug. ***
Comment 7 Jaka Kranjc 2020-09-15 06:16:16 UTC
Hasn't the old language model been replaced already? 

Anyway, in my case I have indentation set to tabs only and KDevelop correctly fixes it for eg. pasted code or when cleaning something with mixed indentation. So it's not at all clear why it thinks it's suddenly ok to use normal spaces.
In these cases there should be none, not even tabs.
Comment 8 Igor Kushnir 2020-09-15 07:51:18 UTC
(In reply to Jaka Kranjc from comment #7)
> Hasn't the old language model been replaced already? 
> 
> Anyway, in my case I have indentation set to tabs only and KDevelop
> correctly fixes it for eg. pasted code or when cleaning something with mixed
> indentation. So it's not at all clear why it thinks it's suddenly ok to use
> normal spaces.
> In these cases there should be none, not even tabs.
The bug is actually in formatting code somewhere, not in language support. I don't know where it is exactly, so I'm changing this bug's Component to "general". If I recall correctly, when I selected Qt Artistic Style and adjusted it to use tabs, this space-inserting issue disappeared.
Comment 9 Piotr Mierzwinski 2020-09-17 14:29:47 UTC
> The bug is actually in formatting code somewhere
> ...
> If I recall correctly, when I selected Qt Artistic Style and adjusted it to use 
> tabs, this space-inserting issue disappeared.

Yes. It might be some workaround this issue, but not solves the problem.
I wonder why in moment of using "Rename assistant" is applying automatic formatting of code, whereas I didn't call it intentionally. If I will want to reformat my code in some style then I will use formatting option in KDevelop menu.

Therefore maybe some solution could be just skipping formatting of code in "Rename assistant", if this is possible. And alternatively adding option in configuration forcing usage of formatting.
Comment 10 Igor Kushnir 2020-09-17 14:52:32 UTC
> Yes. It might be some workaround this issue, but not solves the problem.
> I wonder why in moment of using "Rename assistant" is applying automatic
> formatting of code, whereas I didn't call it intentionally. If I will want
> to reformat my code in some style then I will use formatting option in
> KDevelop menu.
> 
> Therefore maybe some solution could be just skipping formatting of code in
> "Rename assistant", if this is possible. And alternatively adding option in
> configuration forcing usage of formatting.
I am afraid this is not so simple. In many cases KDevelop generates more complex code than simply renaming, and the code path is likely shared by different-complexity refactorings. In case when more code is generated, KDevelop must ensure that it is not formatted in some fixed-for-all-IDE-users style chosen by KDevelop developers, but in the style the user selected for the current session/project. *Disclaimer: I am not sure about this as it has been some months since I dived into the relevant code.*
Comment 11 Piotr Mierzwinski 2020-09-17 17:44:26 UTC
> KDevelop must ensure that it is not formatted in some fixed-for-all-IDE-users 
> style chosen by KDevelop developers, but in the style the user selected for 
> the current session/project.

Yes. I agree, but there are users, like me, who don't use set formatting style, at all. I never called 'reformat code' function in KDevelop. Normally I format code manually, because no one existing (by default in KDevelop) formatting style doesn't match my expectation. So my format of code isn't like KDevelop thinks like it is and in result breaks it. That would mean this isn't a bug and every user should use 'reformat code' before calling "Rename assistant".

Maybe is possible to define own formatting. I mean define where should be space, and where not, or indention in new line, etc.
Comment 12 Igor Kushnir 2020-09-17 18:06:12 UTC
(In reply to Piotr Mierzwinski from comment #11)
> Yes. I agree, but there are users, like me, who don't use set formatting
> style, at all. I never called 'reformat code' function in KDevelop. Normally
> I format code manually, because no one existing (by default in KDevelop)
> formatting style doesn't match my expectation. So my format of code isn't
> like KDevelop thinks like it is and in result breaks it. That would mean
> this isn't a bug and every user should use 'reformat code' before calling
> "Rename assistant".
> 
> Maybe is possible to define own formatting. I mean define where should be
> space, and where not, or indention in new line, etc.
Of course it is possible. There are actually several ways to do this:
1. You can create a new style based on an existing Artistic Style (the *New* button on the Source Formatter tab of KDevelop Settings), then edit the style in a dedicated dialog (the *Edit* button on the same tab).
2. You can switch to a Custom Script Formatter, choose *uncrustify* or *clang-format* and configure these tools via their specific configuration files and command line arguments.
3. There is even a dedicated tool distributed with KDevelop: kdev_format_source. Read the tool's description by selecting *KDevelop: kdev_format_source* on the Source Formatter tab. You can use KDevelop's own formatting configuration as a starting point for your preferred style: https://commits.kde.org/kdevelop/kdevelop?path=format_sources and https://commits.kde.org/kdevelop/kdevelop?path=format.config.uncrustify.

If none of these tools supports your formatting preferences, you can disable formatting altogether by selecting the *KDevelop: kdev_format_source* formatter for the programming languages you use and not providing the kdev_format_source file. In this case no reformatting will be applied to your code.
Comment 13 Igor Kushnir 2020-09-17 18:49:38 UTC
There are screenshots of the Artistic Style customization dialog in the KDevelop user documentation: https://userbase.kde.org/KDevelop5/Manual/Customizing_KDevelop#Customizing_code_indentation
Comment 14 Jaka Kranjc 2020-09-17 19:27:38 UTC
So basically it's an over-engineering problem, ok.
Comment 15 Piotr Mierzwinski 2020-09-18 19:08:22 UTC
(In reply to Igor Kushnir from comment #12)
> (In reply to Piotr Mierzwinski from comment #11)
> >
> > Maybe is possible to define own formatting. I mean define where should be
> > space, and where not, or indention in new line, etc.
> Of course it is possible. There are actually several ways to do this:
> ...
Thanks a lot for suggestions. They were very helpful.
I applied your last advise, where you told how to skip formatting. This was what I needed.
I think I could close this ticket marking it as "Not a bug". Of course only if anybody has nothing against.

BTW.
Before I raised also another bug report (Bug 426642) about 'Adapt signature', where the problem was the same like here - automatic formatting breaks the code. Additionally I mentioned there about some usability in 'Adapt signature' feature.
Anyway also there helped me your advise.
Comment 16 Igor Kushnir 2020-09-18 19:19:59 UTC
(In reply to Piotr Mierzwinski from comment #15)
> Thanks a lot for suggestions. They were very helpful.
You are welcome!

> I applied your last advise, where you told how to skip formatting. This was
> what I needed.
> I think I could close this ticket marking it as "Not a bug". Of course only
> if anybody has nothing against.
I am not sure this is not a bug. Whoever closes it should verify that when a style that matches the actual formatting is selected, the issue is gone. And even if this is indeed the case, I think we should better document when KDevelop applies formatting to the code, and how to configure formatting styles. If we document this well enough in the application itself, we may succeed in preventing such non-bugs from being reported in the future. After all, both you and Jaka were hit by this same issue.
Comment 17 Piotr Mierzwinski 2020-09-18 20:11:56 UTC
(In reply to Igor Kushnir from comment #16)
> (In reply to Piotr Mierzwinski from comment #15)
> > Thanks a lot for suggestions. They were very helpful.
> You are welcome!
> 
> > I applied your last advise, where you told how to skip formatting. This was
> > what I needed.
> > I think I could close this ticket marking it as "Not a bug". Of course only
> > if anybody has nothing against.
> I am not sure this is not a bug. Whoever closes it should verify that when a
> style that matches the actual formatting is selected, the issue is gone. And
> even if this is indeed the case, I think we should better document when
> KDevelop applies formatting to the code, and how to configure formatting
> styles. If we document this well enough in the application itself, we may
> succeed in preventing such non-bugs from being reported in the future. After
> all, both you and Jaka were hit by this same issue.

Just found the issue, where your suggestion about skipping formatting doesn't work how we expected.
Namely, in case of 'Adapt signature', where I supposed that formatting is applied automatically I wanted to skip it so I set kdev_format_source without linked file. And unfortunately formatting happened partially. I mean that in this case indicator of reference or pointer was moved next to type, if originally was present next to name of argument.
I made test with updating definition of method and in declaration these indicators were modified, but space didn't touch.

I think you are right, and we need left this bug report as opened.
Comment 18 Igor Kushnir 2020-09-19 07:18:21 UTC
(In reply to Piotr Mierzwinski from comment #17)
> Just found the issue, where your suggestion about skipping formatting
> doesn't work how we expected.
> Namely, in case of 'Adapt signature', where I supposed that formatting is
> applied automatically I wanted to skip it so I set kdev_format_source
> without linked file. And unfortunately formatting happened partially. I mean
> that in this case indicator of reference or pointer was moved next to type,
> if originally was present next to name of argument.
> I made test with updating definition of method and in declaration these
> indicators were modified, but space didn't touch.
I suppose this is exactly what I surmised in an earlier comment:
> In case when more code is generated, KDevelop must ensure
> that it is not formatted in some fixed-for-all-IDE-users style chosen by KDevelop
> developers, but in the style the user selected for the current session/project.
Probably KDevelop generates new code for your signature in a predefined hard-coded style, then tries to format it in your chosen style and fails, leaving the signature in the hard-coded style. Please try to configure Artistic Style, uncrustify, clang-format or kdev_format_source to match your code formatting as I suggested in my long comment above. See if KDevelop's formatting matches your expectations then. Or is your code formatted so heterogeneously and unsystematically that even fine-grained kdev_format_source configuration is very time-consuming/impossible?
Comment 19 Piotr Mierzwinski 2020-09-21 19:59:03 UTC
(In reply to Igor Kushnir from comment #18)
> (In reply to Piotr Mierzwinski from comment #17)
> > Just found the issue, where your suggestion about skipping formatting
> > doesn't work how we expected.
> > Namely, in case of 'Adapt signature', where I supposed that formatting is
> > applied automatically I wanted to skip it so I set kdev_format_source
> > without linked file. And unfortunately formatting happened partially. I mean
> > that in this case indicator of reference or pointer was moved next to type,
> > if originally was present next to name of argument.
> > I made test with updating definition of method and in declaration these
> > indicators were modified, but space didn't touch.
> I suppose this is exactly what I surmised in an earlier comment:
> > In case when more code is generated, KDevelop must ensure
> > that it is not formatted in some fixed-for-all-IDE-users style chosen by KDevelop
> > developers, but in the style the user selected for the current session/project.
> Probably KDevelop generates new code for your signature in a predefined
> hard-coded style, then tries to format it in your chosen style and fails,
> leaving the signature in the hard-coded style. Please try to configure
> Artistic Style, uncrustify, clang-format or kdev_format_source to match your
> code formatting as I suggested in my long comment above. See if KDevelop's
> formatting matches your expectations then. Or is your code formatted so
> heterogeneously and unsystematically that even fine-grained
> kdev_format_source configuration is very time-consuming/impossible?

Recently discovered another case where was applied any formatting for 'Adapt signature' when I had kdev_format_source without linked file 

Anyway. I will try to configure some formatting tool and will link it to KDevelop. My code is formatted rather systematically, I tried to keep one style in whole code. My style is similar to Artistic Style->Qt only I prefer spaces in decl.and def., which more or less looks like that:

QMap <QString, QStringList> myFoo2( int p, int &r );

int & myFoo1( bool &res, int b, char *c )
{
    int a = 1;
    char *res = nullptr;
    int *r2;
    char tab[5];

    if (a > b)
        myFoo2(b, r2);
    else {
        doSomething(c, b, res);
        return res;
    }
    return r2;
}

Note. This code doesn't make sense, it's only example.

Thanks a lot for advice.