decomp.me icon indicating copy to clipboard operation
decomp.me copied to clipboard

Let users mark notable points in target asm

Open abaresk opened this issue 2 years ago • 10 comments

As you're decompiling, you sometimes have to scroll through the asm to find e.g. branch destinations, register assignments, etc. I often find it easy to lose place of where I was before I started scrolling, especially since asm instructions can all sort of blend together.

It'd be helpful to allow users to set some indicator in the diff-view gutter to mark notable parts of the asm. I'm roughly thinking of a UX that's similar to how VSCode lets you mark breakpoints in code (screenshot), but perhaps using more of an arrow-like shape.

abaresk avatar Dec 10 '22 03:12 abaresk

See also #115

bates64 avatar Dec 10 '22 14:12 bates64

Also sounds very similar to #128

bates64 avatar Dec 10 '22 14:12 bates64

Do you think the owner of the scratch only should be able to mark points on the target asm? Should they persist across all instances of the target asm or just for the scratch in question (e.g. if forked, would they persist)?

I'm thinking that it might work to do something like this, where user avatars are shown: image Then you can click on the avatar to view the comment thread / name of the "notable point"

bates64 avatar Dec 10 '22 14:12 bates64

we were thinking of this initially as a frontend-only feature. but we could add backend stuff to it, just not sure how necessary it is

ethteck avatar Dec 10 '22 15:12 ethteck

and no this shouldn't persist in forks either imo

ethteck avatar Dec 10 '22 15:12 ethteck

I think ideally though you'd be able to set "notable points" without needing to name them, since I think that could disrupt the flow a bit as you're decompiling.

I wonder if perhaps this feature should be separate from the commenting feature? This features only requires a single UI icon per line (it's just quick on/off toggle). But with comments, you might want to eventually support multiple comment threads for the same line. In which case, it might be better for these to be decoupled.

abaresk avatar Dec 10 '22 16:12 abaresk

we were thinking of this initially as a frontend-only feature. but we could add backend stuff to it, just not sure how necessary it is

I think this feature wouldn't be that useful if it were session-only. Navigating away from the page shouldn't clear these

bates64 avatar Dec 11 '22 13:12 bates64

I think ideally though you'd be able to set "notable points" without needing to name them, since I think that could disrupt the flow a bit as you're decompiling.

This makes sense. I would expect naming them would be optional.

I wonder if perhaps this feature should be separate from the commenting feature? This features only requires a single UI icon per line (it's just quick on/off toggle). But with comments, you might want to eventually support multiple comment threads for the same line. In which case, it might be better for these to be decoupled.

The reason I started thinking about comments was that this would share a similar purpose.

We could implement this fairly easily as a small 'toggle your icon for target asm line' feature and later, potentially, develop it into comments. But it need not be comments initially

bates64 avatar Dec 11 '22 14:12 bates64

I'm pretty sure abaresk is mostly concerned with mid-decomping bookmarks, rather than a long-standing bookmark you'll refer back to much later (given my convo with them and the inital description). either way, can't we save the bookmarks with local browser storage?

ethteck avatar Dec 11 '22 14:12 ethteck

either way, can't we save the bookmarks with local browser storage?

This makes sense, we could do it this way for now to skip the backend

bates64 avatar Dec 11 '22 14:12 bates64