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

smali code highlighting is basically perfect #1283

Merged
merged 6 commits into from
Nov 23, 2021
Merged

Conversation

MartinKayJr
Copy link
Contributor

smali highlight is here
#1105
Follow-up to make some improvements to the color matching

@skylot
Copy link
Owner

skylot commented Nov 22, 2021

@MartinKayJr great work!
Several questions:

  • why you don't want to include original jflex file? It will be easier to change/fix token types later
  • did you copy jflex file from somewhere or write it yourself? Just trying to check copyright :)
  • can we use different token types for instructions and type names, like use identifier for instructions? Because now whole line have same color and hard to read:

@MartinKayJr
Copy link
Contributor Author

@skylot hey bro
1.I don’t know where the jflex file should be placed. I put it in the resource directory of the gui in the first submission.
2.This jflex took me 11 consecutive hours to complete and was completed by myself.
3.Regarding the color, I think the RSTA reference manual and the preset Token are not suitable for the smali language, so I am going to optimize the color matching in the follow-up

@MartinKayJr
Copy link
Contributor Author

In my recent submission, I transferred the jflex file to config/jflex and provided a simple shell script for reference to generate SmaliTokenMarker.java.

I tried to use Token.identifier for the instructions , but there was no color, so I changed the fully qualified class name to DATA_TYPE.

@skylot
Copy link
Owner

skylot commented Nov 23, 2021

@MartinKayJr thanks! Looks good.
Although, I think, I will rewrite your shell script using gradle and jflex plugin to make it environment independent.

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.

2 participants