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

Improve jadx #1221

Merged
merged 6 commits into from
Aug 4, 2021
Merged

Improve jadx #1221

merged 6 commits into from
Aug 4, 2021

Conversation

MrIkso
Copy link
Contributor

@MrIkso MrIkso commented Aug 3, 2021

My changes on this project:

  • fixed [core]Line number rendering problem #1153, commit from 4732fa3 dont fix this bug correctly
  • fixed Issues with resource on latest build[core] #1060, styles might contain dots in name
  • fixed skiping only file based resources type. jadx created empty animators.xml ect. It doesn't make sense, because there contains only paths to xml files.
  • code refactored in gui project module
  • use lowercase name on deobfuscated\renamed resources names and id in hexdecimal
  • update all icon to svg format. Used from Intellij Icons. License Apache 2.0, i think there should be no problems

Screenshot

image
image

Copy link
Owner

@skylot skylot left a comment

Choose a reason for hiding this comment

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

Changes like code formatting are necessary for passing build for this PR.
Although, I can fix these issues for you if you have any trouble.

Comment on lines 127 to 132
lineNumbers = new LineNumbers(codeArea);
/* lineNumbers = new LineNumbers(codeArea);
lineNumbers.setUseSourceLines(useSourceLines);
codeScrollPane.setRowHeaderView(lineNumbers);
codeScrollPane.setRowHeaderView(lineNumbers);*/
Copy link
Owner

Choose a reason for hiding this comment

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

Commenting code is not a fix, you just disable important feature (debug line number) instead fixing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll see what exactly it does, I didn't delete it on purpose because I wasn't sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I returned everything back, I thought it was a bug), and this is a feature, and fixed it so that it could only be worked on in decompiled java code, xml file not contains source lines.

Copy link
Owner

Choose a reason for hiding this comment

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

xml file not contains source lines

Actually, AndroidManifest.xml have source lines sometimes. And there is no point in your new changes, please revert them too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AndroidManifest.xml have source lines sometimes

As far as I know, I have never seen them there, it is also turned on by default and I personally do not like it. It can even be rendered into a separate option. This function only bothers me.

Copy link
Owner

Choose a reason for hiding this comment

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

Ok, I see, I can add such option. Just don't disable feature you don't like 😄

@@ -158,20 +158,20 @@ private void search(int direction) {
} catch (BadLocationException e) {
LOG.error("Caret move error", e);
}
}
}*/
Copy link
Owner

Choose a reason for hiding this comment

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

Same here, commenting code is not a fix. If you don't know how to fix this, please open new issue with detail description.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This removes those crutches) from changing the background in the input field. In general, this needs to be rewritten so that it shows which position we are in from the found, like ide search bar.

@@ -119,7 +122,7 @@ private void enableSwitchingTabs() {
lastTab = null;
return;
}
FocusManager.focusOnCodePanel(tab);
// FocusManager.focusOnCodePanel(tab);
Copy link
Owner

Choose a reason for hiding this comment

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

Another code commenting. And I don't see any relation to your changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But this caused a not very pleasant effect when every time a new class is opened when use new themes. Related to improving the ui.

@@ -29,7 +29,7 @@

private static final Gson GSON = new GsonBuilder().create();

private static final ImageIcon ICON = UiUtils.openIcon("icon_quark");
private static final ImageIcon ICON = UiUtils.openSvgIcon("ui/analyze");
Copy link
Owner

Choose a reason for hiding this comment

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

We need to find SVG icon for official Quark icon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Quark have icon? I don't see it on official repo.

Copy link
Owner

Choose a reason for hiding this comment

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

I mean, one on their group logo like it was before.
But I am also unable to find SVG version. So maybe leave PNG version, it looks ok as icon, I think.

Copy link
Contributor Author

@MrIkso MrIkso Aug 3, 2021

Choose a reason for hiding this comment

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

Generated from png logo. Seems to be fine
icon.zip
upaded variant
quark_icon.zip

import jadx.gui.ui.TabbedPane;
import jadx.gui.ui.codearea.ClassCodeContentPanel;
import jadx.gui.utils.OverlayIcon;
import jadx.gui.utils.UiUtils;
import sun.security.mscapi.PRNG;
Copy link
Owner

Choose a reason for hiding this comment

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

Unused import which leads to compilation error.
Please format code before pushing PR, see Code Formatting

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I looked at this, the format of the code in this configuration does not work correctly in my IDEA (2021.2) Windows system. I do not know what to do...

Copy link
Owner

Choose a reason for hiding this comment

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

Just execute gradle spotlessApply.

the format of the code in this configuration does not work correctly in my IDEA

What kind of issue you have?

jadx-gui/src/main/java/jadx/gui/utils/UiUtils.java Outdated Show resolved Hide resolved
@skylot
Copy link
Owner

skylot commented Aug 3, 2021

@jpstotz, @lbj-the-goat can anyone check or review changes to resources processing?

@jpstotz
Copy link
Collaborator

jpstotz commented Aug 3, 2021

@skylot I just checked the commit "fix(res): use lowercase name on deobfuscated\renamed resources names and id in hex format" 6f367d5 except for the code formatting it sounds reasonable to harmonize the resource id to format 0x%08x.

For commit 6bfe4f2 which adds more types to SKIP_RES_TYPES I have to admit that I am not aware why there is such an exclude list and what effects this list has.

@MrIkso
Copy link
Contributor Author

MrIkso commented Aug 3, 2021

commit 6bfe4f2 which adds more types to SKIP_RES_TYPES I have to admit that I am not aware why there is such an exclude list and what effects this list has.

see this or see dumped arsc file using aapt\aapt2 or see file on raw format, or use ApkAnalyser from Android Studio to see arsc file. I watched everything for a long time. Those resources are resource types that in the list SKIP_RES_TYPES contain only paths to files, there are no more values ​​there (except for the id type)

@busmaker
Copy link
Contributor

busmaker commented Aug 4, 2021

@jpstotz, @lbj-the-goat can anyone check or review changes to resources processing?

Hello, Unfortunately I am not familiar with resource formats.😁

@skylot skylot merged commit ee12f0b into skylot:master Aug 4, 2021
@skylot
Copy link
Owner

skylot commented Aug 4, 2021

@MrIkso I merged this PR with following fixing commits. I revert some changes as I requested above.
For line numbers option, I open a new issue: #1223.
Also, I applied your SVG icon for Quark in 667cae2.
If you have any suggestion, you can open new issue or PR.
And thank you for your work! 🎉

@MrIkso
Copy link
Contributor Author

MrIkso commented Aug 4, 2021

@skylot ok, thanks. I still have ideas, upate the search on code and try to close #1212 and #1210 and possibly fix some other problems. With decompilation of java code I cannot help, I don’t understand it... But I will already do this in a new branch and will try to do it not so dirty.

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.

[core]Line number rendering problem Issues with resource on latest build[core]
4 participants