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

add option to set custom source paths #497

Merged
merged 2 commits into from
Jul 21, 2023
Merged

add option to set custom source paths #497

merged 2 commits into from
Jul 21, 2023

Conversation

lievenhey
Copy link
Contributor

this patch add allows the user to add a custom source path which will overwrite the source path provided in the binary
this makes the disassembler, ... work if the source code directory was moved after creating the binary

closes: #418

@GitMensch
Copy link
Contributor

GitMensch commented Jul 14, 2023

Tested with the CI result:

  • opened recording
  • used Disassembly on a function - after a while that was shown, but not source, the source reference pointed to a directory not existing any more
  • gone to settings, added the new source path (select, then add, see it in the list) - Disassembly view was not refreshed
  • used Disassembly on a different function - the source reference still points to the same not existing directory, not the configured one where the source exists
  • used Disassembly on a different function which has its source folder unchanged, works
  • restarted HotSpot, sourcePath was not restored from last session, stopped it
  • started the AppImage ./hotspot-v1.4.1-74-ge66d066-x86_64.AppImage --sourcePaths=/my/source, got an error

    hotspot: Unknown option 'sourcePaths'.

So sadly: not successful.
When it works it seems to like this would close #482 (currently not GH-related).

@lievenhey
Copy link
Contributor Author

thanks for testing. I fixed the --sourcePaths option and added reloading if sourcePaths changes

@GitMensch
Copy link
Contributor

fixed the --sourcePaths option and added reloading if sourcePaths changes

Thanks, this works nearly as expected now.
It seems that while the entries that were included in the GUI are not shown in the configuration after a reload of the application, they are still active (which is not necessarily bad, I actually prefer those to be saved) - maybe they are needed to be included when the setting page is opened?

Not sure how you'd show "persistent" and "per session" entries there (I'd personally mostly need "per session" or "per configuration file", because different projects have the same binary name but the source is in different directories) .

Note that in any case this PR missed to add the option (now also in --help) to https://github.com/KDAB/hotspot/blob/master/README.md#command-line-options

@milianw milianw force-pushed the set-source-path branch 2 times, most recently from f148161 to a25e6c8 Compare July 21, 2023 12:58
this patch add allows the user to add a custom source path which will
overwrite the source path provided in the binary
this makes the disassembler, ... work if the source code directory was
moved after creating the binary

closes: #418
@milianw
Copy link
Member

milianw commented Jul 21, 2023

@GitMensch I think the per-session vs. user settings is a general issue we need to tackle somehow - can you please open a ticket to track that?

the rest of this pull request is now mergeable I believe

@milianw milianw merged commit 35871ad into master Jul 21, 2023
19 checks passed
@milianw milianw deleted the set-source-path branch July 21, 2023 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FR: add option to set source paths
3 participants