[kdevelop] [Bug 358799] Usage of "Update definition signature" in "Adapt Signature" breaks declaration of function if its argument has default value and without it

[Igor Kushnir]

https://bugs.kde.org/show_bug.cgi?id=358799

--- Comment #13 from Igor Kushnir  ---
> Showing a warning is probably more difficult to get right (a nested event 
> loop or code scattered across several functions).
Actually there *is* an easy way to show a notification in the shell embedded
area (the place above the editor tabs where e.g. the "Failed to configure
project ..." error appears when there is a syntax error in CMakeLists.txt).
This can be done via IUiController::postMessage(), MessageType=Information or
MessageType=Positive. But such a message could become annoying once the user
grows accustomed to the `= {} /* TODO */` behavior. So I am not sure it
*should* be displayed.

-- 
You are receiving this mail because:
You are watching all bug changes.

[kdevelop] [Bug 358799] Usage of "Update definition signature" in "Adapt Signature" breaks declaration of function if its argument has default value and without it

[Igor Kushnir]

https://bugs.kde.org/show_bug.cgi?id=358799

--- Comment #12 from Igor Kushnir  ---
One more reason why the current behavior is reasonable: removing the default
parameter value breaks code that uses the function. Adding a new parameter with
a default value keeps the old code working as before. So this second approach
is likely to be used more often by developers - to extend the API without
sacrificing compatibility with previous versions.

-- 
You are receiving this mail because:
You are watching all bug changes.

[kdevelop] [Bug 358799] Usage of "Update definition signature" in "Adapt Signature" breaks declaration of function if its argument has default value and without it

[Piotr Mierzwinski]

https://bugs.kde.org/show_bug.cgi?id=358799

Piotr Mierzwinski  changed:

   What|Removed |Added

 Resolution|--- |FIXED
 Status|REPORTED|RESOLVED

--- Comment #11 from Piotr Mierzwinski  ---
(In reply to Igor Kushnir from comment #10)
> (In reply to Piotr Mierzwinski from comment #9)
> > So the fix breaks different use case.
> But why does it break a use case? Your proposed alternative of removing the
> previous default parameter value can be a loss of information, especially if
> the code in question has not been put under version control yet. Adding a
> frequently valid and reasonable new default parameter value does not lose
> information. The TODO marker suggests the user to consider what the default
> value should be. If the user prefers no default values whatsoever, (s)he can
> remove both the new and the old one easily. Again the TODO marker reminds
> the user about this. I don't think the current behavior is much worse than
> showing a warning. Showing a warning is probably more difficult to get right
> (a nested event loop or code scattered across several functions).

OK. I wonder that showing warning would be good reason, but if you say that it
is probably more difficult to get right then let be right on your side. Your
explanation sounds sensible. I will no longer insist that it is a bug.

-- 
You are receiving this mail because:
You are watching all bug changes.

[kdevelop] [Bug 358799] Usage of "Update definition signature" in "Adapt Signature" breaks declaration of function if its argument has default value and without it

[Igor Kushnir]

https://bugs.kde.org/show_bug.cgi?id=358799

--- Comment #10 from Igor Kushnir  ---
(In reply to Piotr Mierzwinski from comment #9)
> So the fix breaks different use case.
But why does it break a use case? Your proposed alternative of removing the
previous default parameter value can be a loss of information, especially if
the code in question has not been put under version control yet. Adding a
frequently valid and reasonable new default parameter value does not lose
information. The TODO marker suggests the user to consider what the default
value should be. If the user prefers no default values whatsoever, (s)he can
remove both the new and the old one easily. Again the TODO marker reminds the
user about this. I don't think the current behavior is much worse than showing
a warning. Showing a warning is probably more difficult to get right (a nested
event loop or code scattered across several functions).

-- 
You are receiving this mail because:
You are watching all bug changes.

[kdevelop] [Bug 358799] Usage of "Update definition signature" in "Adapt Signature" breaks declaration of function if its argument has default value and without it

[Piotr Mierzwinski]

https://bugs.kde.org/show_bug.cgi?id=358799

--- Comment #9 from Piotr Mierzwinski  ---
(In reply to Igor Kushnir from comment #8)
> The `= {} /* TODO */` part is intentional - implemented in
> https://commits.kde.org/kdevelop/ba2ecbaeba206644c1134569e6da40047096487a to
> fix another bug you reported :). Locations of the feature and the test in
> the current code:
> $ git grep -n '/\* TODO \*/'
> plugins/clang/codegen/adaptsignatureassistant.cpp:213:   
> newSignature.defaultParams[i] = QStringLiteral("{} /* TODO */");
> plugins/clang/tests/test_assistants.cpp:469:<< "class Foo { void
> bar(bool b, int i = 0, char c = {} /* TODO */); };"

So the fix breaks different use case.

-- 
You are receiving this mail because:
You are watching all bug changes.

[kdevelop] [Bug 358799] Usage of "Update definition signature" in "Adapt Signature" breaks declaration of function if its argument has default value and without it

[Igor Kushnir]

https://bugs.kde.org/show_bug.cgi?id=358799

Igor Kushnir  changed:

   What|Removed |Added

 CC||igor...@gmail.com

--- Comment #8 from Igor Kushnir  ---
The `= {} /* TODO */` part is intentional - implemented in
https://commits.kde.org/kdevelop/ba2ecbaeba206644c1134569e6da40047096487a to
fix another bug you reported :). Locations of the feature and the test in the
current code:
$ git grep -n '/\* TODO \*/'
plugins/clang/codegen/adaptsignatureassistant.cpp:213:   
newSignature.defaultParams[i] = QStringLiteral("{} /* TODO */");
plugins/clang/tests/test_assistants.cpp:469:<< "class Foo { void
bar(bool b, int i = 0, char c = {} /* TODO */); };"

-- 
You are receiving this mail because:
You are watching all bug changes.

[kdevelop] [Bug 358799] Usage of "Update definition signature" in "Adapt Signature" breaks declaration of function if its argument has default value and without it

[Piotr Mierzwinski]

https://bugs.kde.org/show_bug.cgi?id=358799

Piotr Mierzwinski  changed:

   What|Removed |Added

 Status|NEEDSINFO   |REPORTED
 Resolution|WAITINGFORINFO  |---

--- Comment #7 from Piotr Mierzwinski  ---
I retested this issue and seems that the problem still persists.
I mean in result I got the same as I described above, so:

void foo(bool param = false, bool a = {} /* TODO */);

Placed in header file.
Of course I'm aware that default value must be last in declaration, but seems
here KDevelop cannot handle this case correctly and breaks code.

Tested with:
KDevelop version 5.9.221170 (22.11.70)

$ apt show kdevelop
Package: kdevelop
Version: 4:22.08.1+p22.04+tunstable+git20221012.0016-0

Operating System: KDE neon Unstable Edition
KDE Plasma Version: 5.26.80
KDE Frameworks Version: 5.101.0
Qt Version: 5.15.7
Kernel Version: 5.15.0-53-generic (64-bit)
Graphics Platform: X11
Graphics Processor: Mesa Intel® Xe Graphics

-- 
You are receiving this mail because:
You are watching all bug changes.

[kdevelop] [Bug 358799] Usage of "Update definition signature" in "Adapt Signature" breaks declaration of function if its argument has default value and without it

[Bug Janitor Service]

https://bugs.kde.org/show_bug.cgi?id=358799

--- Comment #6 from Bug Janitor Service  ---
Dear Bug Submitter,

This bug has been in NEEDSINFO status with no change for at least
15 days. Please provide the requested information as soon as
possible and set the bug status as REPORTED. Due to regular bug
tracker maintenance, if the bug is still in NEEDSINFO status with
no change in 30 days the bug will be closed as RESOLVED > WORKSFORME
due to lack of needed information.

For more information about our bug triaging procedures please read the
wiki located here:
https://community.kde.org/Guidelines_and_HOWTOs/Bug_triaging

If you have already provided the requested information, please
mark the bug as REPORTED so that the KDE team knows that the bug is
ready to be confirmed.

Thank you for helping us make KDE software even better for everyone!

-- 
You are receiving this mail because:
You are watching all bug changes.

[kdevelop] [Bug 358799] Usage of "Update definition signature" in "Adapt Signature" breaks declaration of function if its argument has default value and without it

[Justin Zobel]

https://bugs.kde.org/show_bug.cgi?id=358799

Justin Zobel  changed:

   What|Removed |Added

 Status|REPORTED|NEEDSINFO
 Resolution|--- |WAITINGFORINFO

--- Comment #5 from Justin Zobel  ---
Thank you for reporting this issue in KDE software. As it has been a while
since this issue was reported, can we please ask you to see if you can
reproduce the issue with a recent software version?

If you can reproduce the issue, please change the status to "REPORTED" when
replying. Thank you!

-- 
You are receiving this mail because:
You are watching all bug changes.

[kdevelop] [Bug 358799] Usage of "Update definition signature" in "Adapt Signature" breaks declaration of function if its argument has default value and without it

[Piotr Mierzwinski]

https://bugs.kde.org/show_bug.cgi?id=358799

Piotr Mierzwinski  changed:

   What|Removed |Added

Version|git master  |5.1.2
Summary|Use of "Update definition   |Usage of "Update definition
   |signature" in "Adapt|signature" in "Adapt
   |Signature" breaks   |Signature" breaks
   |declaration of function if  |declaration of function if
   |its argument has default|its argument has default
   |value   |value and without it

--- Comment #4 from Piotr Mierzwinski  ---
(tested on version 5.2.0)

"Adapt signature" called in header breaks declaration and doesn't update
parameter in definition, only updates usage of this parameter.

Header file
  void slotUpdateMatchingStatus( MatchingStatus matchStatus );

After invoking "Adapt signature. Update definition signature"
  void MyClass::slotUpdateMatchingStatus( MatchingStatus eMatchStatus )
  ;

Definition file:
void MyClass::slotUpdateMatchingStatus( FindReplaceTextBar::MatchingStatus
matchStatus )
{
if ( eMatchStatus == Value1 )
do_someting
else
if ( eMatchStatus == Value2 )
do_someting
}

-- 
You are receiving this mail because:
You are watching all bug changes.