Skip to content
This repository was archived by the owner on Apr 16, 2026. It is now read-only.

Address #3695 using data-codemirror-linenumber and CSS content#3696

Open
colelawrence wants to merge 1 commit into
codemirror:masterfrom
colelawrence:issue-3695
Open

Address #3695 using data-codemirror-linenumber and CSS content#3696
colelawrence wants to merge 1 commit into
codemirror:masterfrom
colelawrence:issue-3695

Conversation

@colelawrence

Copy link
Copy Markdown
Contributor

This addresses the open issue: #3695

The code presented is linted, and all tests were passed.

You may view a demo with changes off my gh-pages branch

Please provide feedback on recommended changes or styles.

@danth

danth commented Sep 18, 2017

Copy link
Copy Markdown
Contributor

:lgtm_strong:

Everything looks good to me. All coding styles are the same as surrounding code, there are no obvious errors.


Reviewed 2 of 2 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@adrianheine

Copy link
Copy Markdown
Contributor

This would have to be rebased since I split lib/codemirror.js a year ago.

@marijnh

marijnh commented Oct 20, 2017

Copy link
Copy Markdown
Member

I think this is too invasive. The plan is to eventually, in a breaking version, move all line gutter content to a separate container so that they don't muddle up the code's DOM.

@colelawrence

Copy link
Copy Markdown
Contributor Author

@marijnh I agree that a change like this should probably be considered for a major release as it could break a plugin that depends on reading the DOM structure for line numbers.

I wonder if just using CSS user-select: none could also address this...

@marijnh

marijnh commented Oct 20, 2017

Copy link
Copy Markdown
Member

I wonder if just using CSS user-select: none could also address this...

Unfortunately, no. That'll hide the selection markers, but still copy the text on at least some browsers.

@colelawrence

colelawrence commented Oct 20, 2017 via email

Copy link
Copy Markdown
Contributor Author

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants