Goto line dialog

[leutwe]

[j6t]

The commits have your cryptic github email address as author. Do you insist in using that, or would I be allowed to use your full name and a more useful email address instead?

[leutwe]

I have changed the github setting. Where could I see this cryptic email address?

[j6t]

Type git show on the command line to see your most recent commit. Look at the Author: line.

[leutwe]

Do what you think it is the best.

[j6t]

This Github review system is pure shit. I attempted to comment on 1159675, but it tacked the comment onto the overall conversation.

Do what you think it is the best.

Are you willing to update the commits and adjust them to the project requirements? For example, you should read through existing commits to get to know the commit message style. In particular, there is a summary line, and a justification of the change, i.e., an explanation why the change is needed.

This includes the goal that the commit wants to achieve. Look, for example, at the commit Preferences Back Timeout: 0 has now special meaning. It must explain in the commit message, at best already in the summary line, what the special meaning is.

Also, don't do fixup changes; squash the fixups into the earlier change that introduced the breakage. This concerns the tab conversion fix.

Note also that the indentation style is quite weird. Indentation is 4 positions, but tab width is still 8 and tab characters are used instead of 8 consecutive spaces.

[leutwe]

I'm not so familiar with git.

Do you mean I should make a fresh branch, and replay the commits? Do you prefer to have no tabs in the code?

[j6t]

Given the current state, it is probably easiest to make a new branch and rebuild the commits. But, please, when you are finished, do not make a new pull request, but update this one if possible.

You can also use git rebase -i origin to fix up the previous commits.

You should use tabs in the code, but only when you have two or more indentation levels, because one indentation level (4 positions) is not enough for a tab. Then 3 levels is one tab and 4 spaces, etc.

[j6t]

Thank you for the update.

I like the idea to offer the option that the debugger never goes into the background. But that special value should not be 0, but "infinite". Can we have an edit box with a special value "never"?

I won't take the last commit. I doubt that there is any performance to gain by turning off RTTI. Besides that, the warning that this would remove is more important than to permit custom builds with special compiler options.

Regarding the Goto Line dialog: Good idea. I have a patch sitting around that moves the Find dialog to a toolbar instead of having it as a floating dialog. It would be great to have the Goto line entry in a similar way. But that can possibly be changed later.

[leutwe]

Regarding the Goto Line dialog: Good idea. I have a patch sitting around that moves the Find dialog to a toolbar instead of having it as a floating dialog. It would be great to have the Goto line entry in a similar way. But that can possibly be changed later.

I would also prefer this - but I did not get this to work.

[j6t]

Thank you!

I like the mechanics of the Goto dialog. The implementation needs some polishing, because it does not follow the project style. I would like to point out the details, but the review function on Github are completely incompatible with the way I would like to review commits. (In particular, my comments on an old commit remain even after the code was updated by a later commit, and then the comment is totally outdated.)

• Do not invent your own style to name variables.
• Do not open a block unnecessarily.
• Do not comment on trivial lines of code.
• Please do not write if ( foo ), write if (foo).
• Do not change code that does not need to be changed.

Can we please have a cue text in the edit box of the Goto line dialog, like "Line number". Because on my screen the dialog title is so short that it only says "Goto...". Can it be avoided that there is an own top-level menu that reads "Goto" with a single sub-entry "Goto line number"? I would like to have the "Goto line number" entry in the Edit menu, or perhaps in the View menu.

Can you please rebuild the commits so that it appears that there were no errors made from the beginning? You would use git rebase -i to do that. I mean, do not have "oops, this fixes an earlier error"-commits.

If you think you do not want to do that exercise, I can do it.

And, BTW, do you want to remain anonymous regarding your first name and last name, i.e., do you prefer not to have your name etched into the history permanently? Or would you reveal your name?

[leutwe]

I would prefer if you do the changes you want. in the way you want. I think this will shorten things as not so much recursions are needed.

Regarding the menu entry, my knowledge how the Kde things are working is to low, I didn't get this changed.

[j6t]

I've found a problem with the Goto Line dialog: It should skip disassembly lines, but does not. Can you have a look? Class SourceWindows has lineToRow() that may be helpful.

I have pushed a preliminary clean-up of your work. You can download it with git fetch origin; the branch is origin/goto-line.

[leutwe]

I just saw it today. I will take a look at it.

[leutwe]

Here is a patch which should solve the issue:

#git diff
diff --git a/kdbg/sourcewnd.cpp b/kdbg/sourcewnd.cpp
index 9bc0630..b89554c 100644
--- a/kdbg/sourcewnd.cpp
+++ b/kdbg/sourcewnd.cpp
@@ -346,10 +346,11 @@ void SourceWindow::gotoLine( const QString& text)
	 if (!isSuc)
		return;
 
+       int row = lineToRow(lineGoto);  /*< if assembly is visible row != line. */
	 QTextCursor cursor(document());
	 cursor.setPosition(0);     // goto file first line
 
-    isSuc = cursor.movePosition(QTextCursor::Down, QTextCursor::MoveAnchor, lineGoto - 1);
+    isSuc = cursor.movePosition(QTextCursor::Down, QTextCursor::MoveAnchor, row - 1);
 
	 setTextCursor( cursor );
 }

[j6t]

Thank you for the update. Please give me some time to consolidate the patches.

[leutwe]

Through the change regarding assembly lines for the goto feature, now a assert happens if the specified line number is greater than the line number present in the file.

I added the following at void SourceWindow::gotoLine( const QString& text):

if ( m_rowToLine.size() == 0 )
{
        return;
}
if ( size_t( lineGoto_i ) >= m_rowToLine.size() ) lineGoto_i = m_rowToLine.size() - 1;

before QTextDocument* document_po = document();

But maybe a change of int SourceWindow::lineToRow(int line) would be better.

[j6t]

Please write a new commit with the suggested changes (those from above and these most recent suggested changes and update the pull request. Thanks.

[leutwe]

Hopefully I done the git related things in a usable/understandable way.

[j6t]

Thanks. Will have a look.

[j6t]

I've just pushed out an update of my branch goto-dialog with your latest change squashed in. Do you want to make the changes mentioned in the commit title, i.e.:

• add an OK button to the dialog
• move the menu entry into the View menu (because it does not make sense to have a new top-level menu that has a single entry, even though that would be some standard or convention to have a "Go" menu)