georgebrindeiro / scroll-sync

Atom editor package: Synchronize scrolling of two editor panes by content.
MIT License
29 stars 8 forks source link

scroll-sync enhancements for translation applications #29

Closed juliaanvaneijndt closed 6 years ago

juliaanvaneijndt commented 7 years ago

Hi, I hope I am posting in the right place. I know this package is primarily intended as a kind of quick diff tool for code writing, but I guess it could be quite easily extended to other usages.

For instance, I am using atom to work on a translation in LaTeX. Having two panes opened side by side is very useful to go back and forth from the original to the translation; but I get lost as soon as I have to scroll up and down to check or correct stuff. Each sentence in each file begins with a new line and the same LaTeX command (for numbering purpose): so I thought scroll-sync could use that to keep my panes sync. But it's not the case… Do you have an idea of what is preventing syncing? Is there something I could add to make it work (I tried joining a dummy line of comment before the LaTeX command - with no success)

Here is a minimal working exemple: A-tr.txt A-tx.txt

georgebrindeiro commented 7 years ago

Hi @juliaanvaneijndt ! I actually started using this package for translation as well. Usually I can get panes to sync wherever they match, but I can look into your issue once I'm able to open my computer.

While I did not author the package, one thing I see in the source code (lib/scroll-sync.coffee) is:

      # Make sure that the files are not too much different (at least 20% common lines)
      if n_equal < n_total / 5 then return true

One thing I noticed in your example is that you have no line breaks in the source files... While I did not author the package, I recall from the code that matching is done on a line-by-line basis. So if your whole document is contained in a single line, this criterion will quickly be activated.

juliaanvaneijndt commented 7 years ago

Hi @georgebrindeiro, Thanks a lot for helping! I tried to play with the values in n_equal < n_total / 5, but I guess messing with that would reduce performance. Maybe the line-by-line matching could actually be helpful: for a bunch of reasons, I do not hard-wrap lines, but I do put a line break between each sentences. I could also add matching comments there (updated MWE : Tr and Tx). If it was possible to reduce the scope of comparison to a specific type of lines (let's say those starting with %), that might just be enough…

georgebrindeiro commented 7 years ago

@juliaanvaneijndt Sorry for leaving you hanging. Were you successful in achieving your goal? If so, please share so others can benefit. Also, if you are satisfied with your outcome and don't need any more help, let me know so I can close the issue. Cheers!

juliaanvaneijndt commented 7 years ago

No, unfortunately not. I've tried but I couldn't figure it out. If you have an idea on how to get the comparison to be selective and how to stipulate which lines should be taken into account, your help is welcome. That could actually be a nice option for the package. But if you think this is too much of an user-specific request, feel free to close the issue.

georgebrindeiro commented 6 years ago

I'll leave this request here so I can tackle it when I have some free time, or in case someone wants to take a go at it. I think it's probably doable, but I don't have the time to look into it at the moment.

juliaanvaneijndt commented 6 years ago

Thanks a lot! I'm sorry I can't be of more help on this…

MartinBonner commented 6 years ago

I too want to use scroll-sync to translate - but it my case it is between Java and C++. The code is structurally very similar, but the syntactic differences are enough that very few lines are recognizably the same to a naive diff tool.

A simple, partial, solution would be to turn off trying to match content, and just scrolling both panes by the same number of lines. (That would need frequent breaking out of sroll-sync in order to manually resync).

For my purposes a pair of regular expressions that define "matching" lines would be ideal (I'd match ^@TEST$ to ^TEST_F). That might solve the OP's problem too. (

georgebrindeiro commented 6 years ago

Hi @MartinBonner. I believe that scrolling both panes by the same number of lines is easily doable, if used as an alternative option for the package. Matching regular expressions would be a little harder.

My CoffeeScript experience is limited, but I could attempt both when I have a little more time on my hands. But if you're feeling adventurous, I suggest you fork the repo, make your changes and propose a pull request. That would be very welcome!

juliaanvaneijndt commented 6 years ago

Hi @georgebrindeiro @MartinBonner. Scrolling by line number also seems the best solution to me. Have any of you made progress on that? Atom minimap actually seems to be doing something similar. It may be a good starting point (for someone better at cs than me!)

juliaanvaneijndt commented 6 years ago

I may have made some progress as regards scrolling by line number.

editor = atom.workspace.getActiveTextEditor()
editorEle = atom.views.getView(editor)
currentRow = editorEle.getFirstVisibleScreenRow()
currentLine = editor.bufferPositionForScreenPosition([currentRow, 0]).row

returns the line number of the first row on screen. Then, one can compute the corresponding position in the second pane

otherRow = editor.screenPositionForBufferPosition([currentLine, 0]).row
lineHeight = editor.getLineHeightInPixels()
pos = otherRow  * lineHeight

and then set the scrolling in the second pane

editorElement.setScrollTop(pos)

All of this works fine in the console. Now, I am a bit confused as to how to integrate it into the existing code… Would you know how to do it? I tried to figure it out, but it keeps breaking. I guess it should be something like this:

# First line from first row in current pane
thisRow = thisInfo.editorEle.getFirstVisibleScreenRow()
thisLine = thisInfo.editor.bufferPositionForScreenPosition([thisRow, 0]).row

# Find the corresponding row in the other pane
otherRow = otherInfo.editor.screenPositionForBufferPosition([thisLine, 0]).row

# calculate position
pos = otherRow  * @lineHeight

# Scroll the other pane
otherInfo.editorEle.setScrollTop(pos)
juliaanvaneijndt commented 6 years ago

Ok I found the solution. I forked the package: let me know if it works for you. I would have created a pull request, but it no longer scrolls by content, only by line number. I still think it would be great to make an option out of it in the original package.

georgebrindeiro commented 6 years ago

@juliaanvaneijndt Hi Juliaan, nice to hear! Sorry I didn't reply sooner, but I was really invested in my recent wedding and didn't really pay enough attention to Github. I'll have some vacations soon and I think I can include that option in the package, from the changes I have seen. For now, I will close this ticket and open a new one, which more accurately describes the work to be done. Nice job!