Select to the only bookmark

[alefragnani]

Requirements

• Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
• All new code requires tests to ensure against regressions

Description of the Change

This PR adds update the Select to Next Bookmark and Select to Previous Bookmark, to work even if the only bookmark is positioned in the other direction

Alternate Designs

It follows the same behavior of Jump to Next/Previous Bookmark commands.

Benefits

You can take advantage of the selection commands even you don't know exactly if the cursor is above or below

Possible Drawbacks

Nothing at all, since it follows the same behavior as Jump commands

Applicable Issues

This PR closes #94 .

[lee-dohm]

The one question I have here is around directionality of the operations. While it makes sense to make "Select to ..." and "Jump to ..." operations work equivalently, selecting a range adds another wrinkle to things. For example, take the following file contents:

1
2
3
4
5
6
7
8
9
0

Let's assume that the cursor is at the beginning of the line containing 8 and the bookmark is on the line contaniing 2. If one executes the command "Jump to next bookmark", it doesn't matter which direction the cursor travels to get there. It doesn't matter if it travels backwards through the file from 8, to 7, to 6, etc, onwards to 2 or if the cursor travels forwards through the file and wraps around from 0 to 1, to 2.

But when executing "Select to next bookmark", directionality does matter. If I am at the line containing 8 and I execute "Select to next bookmark", I would expect the selection, if any, to go from the current location toward the end of the file. But it appears this change makes the selection go from the current location toward the beginning of the file. Is my understanding of this change correct? If so, I don't believe that this is the correct behavior, even when it is the only bookmark in the file.

[alefragnani]

Hi @lee-dohm ,

I understand your point, but this PR was created to fix #94, which was created almost an year ago and labeled as bug and triaged because the Jump to ... and Select to ... commands weren't consistent.

I think that the reason for that is: The user should not worry if the unique bookmark is above or below the current cursor position. He/She would like to Jump to / Select to it. At this point, the Jump to command is moving the cursor in the oposite direction (because it does a circular movement) but not the Select to command. That's what I understand to be the bug.

Do you think this behavior (Jump to / Select to) with only one bookmark should have a setting, to define if the user would like this circular movement?

Thanks

[lee-dohm]

If, as you say, this is intended to create circular movement, or the illusion thereof, shouldn't two selections be created then in my scenario above?

1. From the beginning of the line with 8 through the end of the line with 0
2. From the beginning of the line with 1 through the beginning of the line with 2

If your proposed change creates only one selection as described in my previous comment, why do you believe that is more correct behavior than the two selections I describe here? How would that represent circular movement?

[alefragnani]

If your proposed change creates only one selection as described in my previous comment, why do you believe that is more correct behavior than the two selections I describe here? How would that represent circular movement?

I don't think it is more/less correct, than your proposal, this was just my interpretation of the expected behavior described in the original issue.

Expected behavior: All the lines between where the cursor was the momento I pressed F2 and where the bookmark is should be selected.

As I said, I think the reason was the user does not worry where the bookmark is located, it just selects to it. IMHO, create two selections is a bit weird, but it's just my point of view.

I think it would be a good idea to ping @alexandernst (which created the original issue), and @rsese (which labeled bug and triaged), so they could also describe their opinions.

What to you think?

[lee-dohm]

@alefragnani Well, you've already taken care of pinging the correct people :grinning:

FYI @rsese and I work on the same team and were in the same meeting with other Atom team members when we discussed the question of what should be the correct behavior. We all had the same concerns after the discussion, hence the reason why I asked the questions. Additionally, I'm the one that created the triage process for the Atom team, so I'm more than familiar with how we use the triaged and bug labels, neither of which preclude asking questions to ensure that the correct implementation is reached.

Also, while @alexandernst created the issue, the Atom team are the ones that are going to have to maintain the implementation going forward and answer questions as to why things are implemented the way they are when someone disagrees with the decisions that were made. So the Atom development team, of which I am a part, will need to make a final determination on what most people will expect the behavior to be, even if that differs from the original issue description.

I hope that makes things a bit more clear and eases your concerns and frustration.

[alefragnani]

@alefragnani Well, you've already taken care of pinging the correct people 😀

😆

I hope that makes things a bit more clear and eases your concerns and frustration.

Don't worry. There is no concern, nor frustration. I know you from the very beginning of Atom. (If I remember correctly, you were an avid forum's user and based on your engagement, you were contacted by the Atom's team to be part of the project). Even if my memory is playing with me, I'm sure that if you ask more info about some issue, you have a good reason to do so 😄 .

I'm contributing to the project because I felt confident about the original issue, and noted that I could contribute a bit more to the project. I'm open to update the PR if necessary.

In fact, I would like to bring some of the features of my Bookmarks extension for VS Code, but of course, only if you (the maintainers) would like to receive. I noted some open issues that I could contribute as well.

Thank you

[alefragnani]

Hi @lee-dohm, any news about it?

Would you like me to update the PR in order to create two selection, as you described. Is there any additional change that you would like me to do?

Thank you

[lee-dohm]

No news currently. It's awaiting triage by the dev team.

[alefragnani]

That's great @maxbrunsfeld, thank you 👍

[alefragnani]

🎉