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 a Code Model #30

Merged
merged 1 commit into from
May 6, 2024
Merged

Add a Code Model #30

merged 1 commit into from
May 6, 2024

Conversation

alonalbert
Copy link
Contributor

This makes it easier to detect jumps and toggle show-line-number

Copy link
Owner

@romainguy romainguy left a comment

Choose a reason for hiding this comment

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

Thanks! In addition to the comments I left, I noticed a huge difference in code formatting:

  • Without line numbers the indentation is now 2 spaces, please keep the original, it's much more readable to me
  • With line numbers on, the indentation is very wide (12 spaces??) with a lot of empty space around the line numbers. See screenshot
Screenshot 2024-05-04 at 7 31 29 PM

@alonalbert alonalbert force-pushed the refactor-code branch 3 times, most recently from ded97c2 to 1b9d157 Compare May 5, 2024 14:52
@alonalbert
Copy link
Contributor Author

Regarding the indentations, I changed things up a bit.

Note that we have to account for long line numbers so:

class Foo
    <init>()V // Foo.<init>()
        1:    0000: invoke-direct {v0}, Ljava/lang/Object;.<init>:()V // method@0008
              0003: return-void

Might seem to sparse, but when line numbers get big:

    <init>()V // Foo.<init>()
        1000: 0000: invoke-direct {v0}, Ljava/lang/Object;.<init>:()V // method@0008
              0003: return-void

It makes sense.

I have a plan to use the max line number to derive the width, but will leave this to a future cl.

@alonalbert
Copy link
Contributor Author

FYI, the OAT parser has a bug in ti. Infinte loop sometimes
I'm investigating

@alonalbert
Copy link
Contributor Author

OK, fixed it.

@romainguy
Copy link
Owner

Note that we have to account for long line numbers so

That's what I thought. Have you tried eating up into the left indent instead? I don't know how pretty it would look but something like this:

class Foo
    <init>()V // Foo.<init>()
1:      0000: invoke-direct {v0}, Ljava/lang/Object;.<init>:()V // method@0008
        0003: return-void
    <bar>()V // Foo.<init>()
1000:   0000: invoke-direct {v0}, Ljava/lang/Object;.<init>:()V // method@0008
        0003: return-void

This makes it easier to detect jumps and toggle show-line-number
@romainguy romainguy merged commit 1a4b8bc into romainguy:main May 6, 2024
1 check passed
@alonalbert alonalbert deleted the refactor-code branch May 8, 2024 13:49
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