atom / bookmarks

Bookmark editor lines in Atom
MIT License
49 stars 36 forks source link

Make bookmarking possible with multiple cursors #64

Closed solendil closed 8 years ago

solendil commented 8 years ago

There was an attempt in the code to implement multiple cursors (related to #59)

for cursor in cursors       
  range = @editor.getSelectedBufferRange()       
  bookmarks = @markerLayer.findMarkers(intersectsBufferRowRange: [range.start.row, range.end.row])             

I figured I could make it work, so here's the fix, along with its jasmine spec.

winstliu commented 8 years ago

Can you also add a spec for simultaneously adding and removing bookmarks?

winstliu commented 8 years ago

Interestingly, intersectsBufferRowRange doesn't seem to be a valid option. The closest there is is intersectsRowRange. https://github.com/atom/text-buffer/blob/v8.5.0/src/marker-layer.coffee#L94

solendil commented 8 years ago

@50Wliu I removed the unused line of code and changed intersectsBufferRowRange to intersectsRowRange. How did it work? I'm not sure, but it still does.

As for the specs, there are

describe "multiple point marker bookmark", ->
    it "creates multiple markers when toggled", ->

and

 it "removes multiple markers when toggled", ->

Do you think more is needed?

winstliu commented 8 years ago

Yeah, I think it's best to make sure that adding and removing at the same time works as well.

solendil commented 8 years ago

Here it is : this new spec tests the mechanism in some extreme scenario. It seems unlikely that anybody will ever try that in real-life, but if they do, it will work :)