Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Implement ranged single- and multi-line bookmarks#47

Merged
kuychaco merged 4 commits intoatom:masterfrom
Tyriar:9_ranged_bookmarks
Feb 18, 2016
Merged

Implement ranged single- and multi-line bookmarks#47
kuychaco merged 4 commits intoatom:masterfrom
Tyriar:9_ranged_bookmarks

Conversation

@Tyriar
Copy link
Contributor

@Tyriar Tyriar commented Oct 26, 2015

Have bookmarks remember their column and row instead of just row and also keep
track of their ending position.

Fixes #9

Jumping forward/back:

bookmarks_jump

Toggling:

bookmarks_toggle

Bookmark view with multi-line bookmarks:

bookmarks_view

Have bookmarks remember their column and row instead of just row and also keep
track of their ending position.

Fixes #9
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that there's bookmarkEndRow, I think it would be better to explicitly name this bookmarkStartRow.

@Tyriar
Copy link
Contributor Author

Tyriar commented Jan 31, 2016

Ping?

@lee-dohm lee-dohm added the atom label Feb 1, 2016
atom.commands.dispatch editorElement, 'bookmarks:toggle-bookmark'

editor.setCursorBufferPosition([10, 0])
editor.setSelectedBufferRanges([[[8, 4], [10, 2]]])
Copy link
Contributor

Choose a reason for hiding this comment

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

Since line 10 is empty, could you change this range to end at [10, 0]? This would then be consistent with the tests on lines 217, 231, 237, and 242.

@kuychaco
Copy link
Contributor

@Tyriar this looks great! I added a few minor comments to the diff. Once you take care of those I'll gladly merge this PR. Thanks for the good work.

@Tyriar
Copy link
Contributor Author

Tyriar commented Feb 18, 2016

@kuychaco done

kuychaco added a commit that referenced this pull request Feb 18, 2016
Implement ranged single- and multi-line bookmarks
@kuychaco kuychaco merged commit c157f1c into atom:master Feb 18, 2016
@kuychaco
Copy link
Contributor

Sweet, thanks @Tyriar 👍

@CyberPunkCodes
Copy link

This PR doesn't make any sense. Without the selected sections being highlighted, it's pointless. All you get is a range in your bookmarks list (ie: atom-environment.coffee:9-11). Unless your lines are a million characters long, it isn't hard to spot what you need on a bookmarked line.

From the editor view, it does the same thing as adding multiple bookmarks at the same time (ie: bookmarking line 4, 5 and 6).

If this PR is stopping us from having the entire bookmarked line itself highlighted, then I vote for reversing this PR.

Having the highlighted line is way more important (and desired feature) than this! When you have a really long file full of code, it makes it very helpful to have the lines highlighted so you can spot them when scrolling through it. It's nearly impossible to spot the tiny bookmark icon in the gutter when scrolling through a 5,000 line file.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants