Skip to content

End of line to end of screen line - #2

Open
robbertvs wants to merge 3 commits into
masterfrom
end-of-line-to-end-of-screen-line
Open

End of line to end of screen line#2
robbertvs wants to merge 3 commits into
masterfrom
end-of-line-to-end-of-screen-line

Conversation

@robbertvs

Copy link
Copy Markdown

For discussion, do not merge.

@rbottema
rbottema force-pushed the end-of-line-to-end-of-screen-line branch from 0738985 to f9b1652 Compare March 14, 2016 08:53
@robbertvs

Copy link
Copy Markdown
Author

I talked to Lee Dohm and he said it would not be good to change the function names. He said I should just change register-default-commands. If I would only change this, the register command names would not be consistent with the API.

I think the API should be changed to make the function names consistent.
What do you think?

@acontreras89

Copy link
Copy Markdown

Oh, so someone is already working on atom#7159... I was about to open my own PR 😅

I agree we should rename function names, because we definitely want to have functions/commands for both, selecting to the end of line and to the end of screen line.
I can definitely see the cons of performing such change, but I believe it's necessary to keep things consistent and legible. My opinion may be biased, though, as I am just a contributor.

Also, I believe this PR should also take care of the 🐛 with the end of screen line leaving one character left. Can you take a look at it, @robbertvs? Note that this only happens when one token is split among several lines.

@robbertvs

Copy link
Copy Markdown
Author

@acontreras89 I agree we should look at the fact that one character is left should be looked at as well, though I think that can be done in a separate pull request because they are different problems concerning different parts of the code. (also changing the API might need some discussion before it can be merged, but correcting the end-of-screen-line selection can be merged immediately.)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants