Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix source code search #595

Merged
merged 3 commits into from
Apr 23, 2024
Merged

fix source code search #595

merged 3 commits into from
Apr 23, 2024

Conversation

lievenhey
Copy link
Contributor

The search functions only works on a limited range, but the model contained the complete source code. This patch runs the search function on only a subset of that code.
fixes: #577

@GitMensch
Copy link
Contributor

GitMensch commented Jan 16, 2024

Is there any option to adjust the search model test for that case?

Copy link
Member

@milianw milianw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also appreciate a better test coverage to handle this ideally

src/models/sourcecodemodel.cpp Outdated Show resolved Hide resolved
src/models/sourcecodemodel.cpp Outdated Show resolved Hide resolved
@lievenhey
Copy link
Contributor Author

I rewrote the search functions to use iterators instead + added some tests

@lievenhey lievenhey added the bug label Feb 6, 2024
@GitMensch
Copy link
Contributor

Looking forward to get this merged.

The search functions only works on a limited range, but the model
contained the complete source code. This patch runs the search function
on only a subset of that code.
fixes: #577
@lievenhey lievenhey force-pushed the fix-search branch 10 times, most recently from 32067fe to 044d9f1 Compare April 17, 2024 11:36
Copy link
Member

@milianw milianw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one minor nit then this can go in

thank you!

tests/modeltests/tst_search.cpp Outdated Show resolved Hide resolved
@GitMensch
Copy link
Contributor

GitMensch commented Apr 23, 2024

I've pot some time into testing the appimage - the source handling in the Disassembly window now seems to work fine.

The only thing that feels strange is the go to function:

  • seems to be only usable with the context menu
  • CTRL+G is identical to ALT+G and selects/closes the Flame Graph; I guess that's the QT default we want to keep *but maybe CTRL+SHIFT+G can then open the go to dialogue as well

And of course #523 and #528 as well as #607 would be very helpful.

Until now search() works on an array like object. This works fine if the
whole array is searched. But if only a part is searched, this creates an
unnecessary copy.
@lievenhey lievenhey merged commit 1fcf0ae into master Apr 23, 2024
25 checks passed
@lievenhey lievenhey deleted the fix-search branch April 23, 2024 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disassembly displays old source (caching issue?) and search is broken
3 participants