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

Decorate bookmarked lines#67

Closed
solendil wants to merge 3 commits intoatom:masterfrom
solendil:bookmark-decorate-line
Closed

Decorate bookmarked lines#67
solendil wants to merge 3 commits intoatom:masterfrom
solendil:bookmark-decorate-line

Conversation

@solendil
Copy link

@solendil solendil commented Jun 18, 2016

As per request #33, this commit adds the .bookmarked class to .line as well as .line-number elements.
This allows to decorate lines. For example:

atom-text-editor::shadow .line.bookmarked {
  background-color: rgba(0, 153, 204, 0.3);
}

image

@solendil
Copy link
Author

solendil commented Jun 18, 2016

The appveyor build failed due to an internal error, not because of tests related to this commit.
How can I relaunch the build? Should I make a fake commit?


waitsFor ->
lines = editorElement.shadowRoot.querySelectorAll('.bookmarked')
editorElement.shadowRoot.querySelectorAll('.line.bookmarked').length is 1
Copy link

Choose a reason for hiding this comment

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

This line doesn't seems to do anything. Same goes for a few more below.
To my understanding CoffeeScript returns the value from the last line within a function, defining two (or more) lines will still only return the last line.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for your keen eye, @jerone
You're right, my specs were not catching problems related to .line.bookmarked items
It should be fixed now

@winstliu
Copy link
Contributor

I've restarted the Appveyor build.

@winstliu
Copy link
Contributor

As I recall, bookmarks can now be selection-based (#47), so giving the entire line the bookmarked class doesn't seem like the right thing to do.

@solendil
Copy link
Author

I believe many people think that Atom bookmarks is a line-only feature : when you define a bookmark at the cursor, it places a symbol in the gutter for the current line. Nice and sweet and useful.

I, for one, did not know that bookmarks were applied to range until I delved in the code and discovered this feature. It is powerful, but mildly confusing, and I still do not use it in my everyday work: to me, bookmarks are still a line-only feature.

Since this PR is aimed at providing classes that can be used, but are not mandatory, I believe it makes sense to offer a .line.bookmarked class for people or add-ons who want to decorate bookmarks on a line-basis.

I also added a .bookmarked class that allows to decorate the bookmarked selection. Here is an example of style

atom-text-editor::shadow .highlight.bookmarked .region {
  background-color: rgba(0, 153, 204, 0.3);
}

And the result

image

@CyberPunkCodes
Copy link

This should have been in atom since day one. If you can add a bookmark, and style the gutter line number section, you should be able to style the line itself. It seems like a basic and common necessity for an editor like Atom, which prides itself of flexibility and customization through a wide array of themes and packages. Yes, we can create a package to "hack it", but we shouldn't have to. The "bookmarked" class on the gutter and not on the line number, just shows someone only thought out the bookmark process half way. Seriously, a few lines and push the update. It would take only a few minutes to add this feature baked into the next Atom release.

I CAN NOT BELIEVE that this has been going on for over a year, and it still isn't built into Atom. It is a huge let down. I hate seeing something so easy, simple, and duh should have been there from the beginning, get ignored!

This issue has fallen through the cracks, and someone responsible for Atom's updates/code fixes/modifications should add it.

@CyberPunkCodes
Copy link

I want to add, that I manually added the Atom "bookmarks" package via apm develop bookmarks, and added these changes from this PR manually. Then of course, wrote my own style in styles.less.

It works just fine, no issues.

This looks super simple, and mimics exactly how the bookmarked class is added to the gutter line. I can't fathom how there would be any issues with these few minor lines.

This PR should be merged.

@CyberPunkCodes

This comment has been minimized.

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.

4 participants