code-peek-atom icon indicating copy to clipboard operation
code-peek-atom copied to clipboard

Peeked code only scrolls to top of function

Open martindsouza opened this issue 8 years ago • 11 comments

When you peek into a function the "peeked window" only starts at the current method. In other systems (VSC) you can scroll forwards and backwards in peeked code.

martindsouza avatar May 08 '17 21:05 martindsouza

You're right. It was always my intention that it only showed that specific function, though. I'm pretty sure that's how Bracket's quick edit feature worked, which is what I was initially trying to emulate.

I'll look into having the whole file be available with it automatically scrolling to the start of the function. If I get it to work, I'll probably make that a boolean configuration setting.

Thanks for the feedback. It wasn't something I considered.

DFreds avatar May 09 '17 17:05 DFreds

@DFreds This may also keep things easier than your original intention of finding the end of a function. Overall I won't create this as an option and just make it the default. Since you move the pointer to the top of the function a user has to manually scroll up to go above thus showing their intention. I don't see why someone would turn this off.

martindsouza avatar May 11 '17 14:05 martindsouza

Maybe I'm misunderstanding something. Do you mean that it only shows the function you peeked in the panel? And that you want it to let you scroll through the entirety of the file it peeked as well? That's what I assumed you meant.

I always had the intention to only show the function you wanted to see. There exists the blue code icon button that will open the other file in a new tab.

DFreds avatar May 11 '17 19:05 DFreds

This is what VSC does and I really like it. I acknowledge your perspective on only showing the function but it causes some issues:

  • You miss the function's comments above the function initial declaration. (code-peek vs VSC: I can scroll up a bit and see all the functions comments in VSC).
  • You'll need to also require the regexp to end the function which may not be worth the hassle. In the long run.

vsc-peek

martindsouza avatar May 11 '17 19:05 martindsouza

I would also be in favor of peeking through the whole file, but having the cursor at the top of the current method.

vincentmorneau avatar May 11 '17 19:05 vincentmorneau

Hmm, you make a good point. It'll be a pretty significant change in the code, but I think it might actually make it easier in the long run because I won't have to worry about when the function actually ends. Thanks for the suggestion, I'll take your suggestion and work on it.

DFreds avatar May 11 '17 20:05 DFreds

So I tried working on this and refactored a lot of code, but I'm having an issue where the code peek panel won't scroll down to the start of the function. It's only noticeable when the file is sufficiently long enough (like javascripttest.js). If anyone wants to help me figure out why that'd be awesome!

I've basically tried every scroll method in the Atom API: https://atom.io/docs/api/v1.16.0/TextEditor#instance-scrollToBufferPosition

WIP pull request is here: #35

DFreds avatar May 12 '17 01:05 DFreds

@DFreds depends. Some devs use tabs and others use spaces. I use spaces. I think a lot of this will be resolved with #32 as you suggested in #29 where we don't focus on finding the end of the function.

martindsouza avatar May 12 '17 13:05 martindsouza

Actually, tabs and spaces wouldn't actually matter here. I was just curious if you HAD to use consistent tab/space spacing like in python.

DFreds avatar May 12 '17 17:05 DFreds

@tschf since you've done some Atom development any ideas to help with the scrolling issue?

martindsouza avatar May 16 '17 04:05 martindsouza

I honestly don't even know if it's strictly an Atom issue. I get the feeling that Atom somehow "thinks" the panel is on screen and therefore doesn't scroll. It might be inherent to how panels are rendered. This might be something that could be fixed by javascript or jquery, but I'm not sure. The built-in scroll methods in the Atom API do not work, though.

DFreds avatar May 16 '17 19:05 DFreds