Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

editor.getUserLineSelection() and editor.getUserCharSelection() should allow other defaults if empty selection #304

Open
victorel-petrovich opened this issue Aug 16, 2023 · 13 comments

Comments

@victorel-petrovich
Copy link

victorel-petrovich commented Aug 16, 2023

... because it's not always the case that the whole document is needed if nothing is selected. For example, current line could be considered instead.

So I suggest for these functions to instead return same values for startLine, endLine and startByte, endByte, corresponding to the current position of the caret.
Then code could decide what to do next.

EDIT: or, at least, provide an optional 3rd argument, wholeDocIfEmptySel=True, boolean.

Thanks.

@Ekopalypse
Copy link
Contributor

To avoid corrupting existing code, a third parameter such as curLineIfEmptySel=False should be used instead.

@victorel-petrovich
Copy link
Author

victorel-petrovich commented Aug 17, 2023

@Ekopalypse , what I suggested in my EDIT above would be clearer (I've added now there True):
whether or not when empty selection to give the values for the whole document.
With your suggestion, it will not be clear what it will do when your param is False.

Also, considering current line is, again, just one possibility. User might want to just do nothing if selection is empty -- and to tell that selection is empty, the returned values should correspond to empty selection, not for a line.

@Ekopalypse
Copy link
Contributor

@victorel-petrovich
I think we all agree that no existing code should be broken when a change is introduced. So the current behavior should remain as it is.
This means that both functions act either on the current selection or, as documented, on the entire document.
To give the user the possibility to react to something else, I think it would be best to introduce a new parameter that defines what it does when set. In order not to break the existing code, it must be inactive by default. This is what curLineIfEmptySel=False stands for. The document would then mention that if you want to get the current line instead of the whole document, you have to call it with curLineIfEmptySel=True.

We seem to have a different view in general about what a user can expect. For me, functions can/should only provide what they are supposed to do and what is documented. If I use getUserLineSelection and it is documented that it acts on the entire document, if nothing is selected, then so be it. There is a getSelectionEmpty function to find out if there is a selection.

I agree that it is not intuitive to use the entire document when nothing is selected if we were to introduce this as a new function, but since it was introduced this way and, more importantly, it was documented, I tend to say that it should not be changed.

Personally, I think it's better to introduce a new parameter that also does new things instead of confirming old things that have always been done ... but these are personal preferences.
A new parameter curLineIfEmptySel, set to False by default, is easier for me to understand than wholeDocIfEmptySel=True. With wholeDocIfEmptySel, the question will arise with both parties, old and new users, and what happens if this is set to False?
But if I read curLineIfEmptySel=False and think about what curLineIfEmptySel=True could mean I would intuitively say, ok ... if the parameter is True then I get the current line as start and end lines returned.

Just to avoid confusion, I don't care in the end what the parameter is called, the only important thing from my point of view is that existing code still runs after a change.

@victorel-petrovich
Copy link
Author

victorel-petrovich commented Aug 18, 2023

I think we all agree that no existing code should be broken when a change is introduced. So the current behavior should remain as it is.

Yes, that was clear from your first comment.

For me, functions can/should only provide what they are supposed to do and what is documented.

Fact ! :)

With wholeDocIfEmptySel, the question will arise with both parties, old and new users, and what happens if this [ wholeDocIfEmptySel ] is set to False?

-- what the function is supposed to do: which is simply to return the startLine, endLine and startByte, endByte of the current selection.
An empty selection is empty, that is still a selection (according to Scintilla), that starts and ends at the current position of the carret. Thus what it should return is clearly: current line number and current caret position, each duplicated, respectively.

But it can be documented as well, if you say that is not self-evident.

And, as I mentioned, the other advantage IMO is that it's more general; it leaves up to the user to decide whether to consider current line or current word something else entirely in case wholeDocIfEmptySel=False

Just to avoid confusion, I don't care in the end what the parameter is called, the only important thing from my point of view is that existing code still runs after a change.

👍

@victorel-petrovich
Copy link
Author

victorel-petrovich commented Aug 18, 2023

On a bit more thinking on what could be reasonable to return when selection is empty:

While for editor.getUserLineSelection() , it's either current line values, or first and last lines in doc, thus Boolean seems enough for the 3rd argument,

For editor.getUserCharSelection(), could be: current position, or start and end pos. of current line , or start and end pos. of doc;, or (who knows ?). So for it, a type such Integer could be better, with possible values:

  • -1 => start and end pos. of doc
  • 0 => current position

This will allow extending the function when users find the need, to say a 3rd value: 1, to return start and end pos. of current line

@alankilborn
Copy link

editor.getUserLineSelection() ... should not assume whole document if empty selection

To this whole idea, I say: Why not assume the whole document?

In Notepad++ (disregarding PythonScript entirely), if you use the line sorting functions without having a certain set of selected lines, what happens? The entire document gets sorted. So...there is some precedent for the way this PS function works. And the logic of it is "not bad". And it is well-documented. And, as was pointed out, editor.getSelectionEmpty() can also be used in order to avoid the logic that isn't liked (by one person) here.

IMO, don't fix what isn't broken. Also, don't complicate what isn't broken.

@victorel-petrovich
Copy link
Author

I already brought arguments for why , from beginning.

if you use the line sorting functions without having a certain set of selected lines, what happens?

But what happens if you use delete operation? Or if you use On selection > Search on Internet?

etc

in order to avoid the logic that isn't liked (by one person) here.

Too early to conclude how many are for or against.

@victorel-petrovich victorel-petrovich changed the title editor.getUserLineSelection() and editor.getUserCharSelection() should not assume whole document if empty selection editor.getUserLineSelection() and editor.getUserCharSelection() should allow other defaults if empty selection Aug 18, 2023
@alankilborn
Copy link

But what happens if you use delete operation? Or if you use On selection > Search on Internet?

These are not line-based operations.

Too early to conclude how many are for or against.

Not really -- the current functionality has been in place for a long time with no complainers.

@victorel-petrovich
Copy link
Author

victorel-petrovich commented Aug 18, 2023

That's partly the point, it doesn't have to be about line-based operation (however you define them).
Also, 2nd function in title is not about lines at all.
Both are about selection.

I really meant how many for/against the Issue I made.

@alankilborn
Copy link

I really meant how many for/against the Issue I made.

You've probably gotten all the people that will "vote" already.

@alankilborn
Copy link

Have you really encountered "problems" with the way these functions are through writing scripts and being surprised by the functionality?

You strike me as someone that is just going along and reading the API and finding things to complain about.

We get such complainers here occasionally; not sure where such behavior originates...

Anyway, you'll get responses from those such as @Ekopalypse and myself, but the maintainer of PythonScript (@chcg) and even Notepad++ (Don Ho) won't bite, and they will probably just put the big REJECT on your ideas.

BTW, your idea is not terrible, but the time to implement it would have been with the first incarnation of the function. Subsequent to that, it's a big "who cares?", and other scripting constructs are used to avoid the "whole doc" functionality when desired, as previously mentioned. It's simply not important enough to change, even with the idea of another argument.

@victorel-petrovich
Copy link
Author

victorel-petrovich commented Aug 19, 2023

You strike me as someone that is just going along and reading the API and finding things to complain about.

Not complaining, but striving for improvement, wherever possible.

What's weird is that you're soo insistent in fighting this issue, as if the particular matter here is that important to you.

and they will probably just put the big REJECT on your ideas.

I'm not worried about that.
But it seems you'd like that.
It's like I have rubbed you the wrong way at some point - I have almost no idea when/how - and you decided to "come after me". I can assure you you're making a mistake - I didn't have but respect for you up to this point.
In fact, it's due to you that I learned a tiny bit of scripting here.

Subsequent to that, it's a big "who cares?",

Exactly. Then why don't you just ignore it as well...

@alankilborn
Copy link

What's weird is that you're soo insistent in fighting this issue, as if the particular matter here is that important to you.

My goal in commenting in this issue is simply to "vote" (express my opinion). I'm only "fighting" it because it isn't a necessary thing. If it gets implemented, I'm not going to be unduly upset.

But I'd rather the devs spend their volunteer time on other things. And...if people stay silent, a rogue person could drive a bad change (that's "in general", not specific to this one) -- this has happened.

It's not personal, of course. If you have good ideas, I'll let you know; same for what I consider bad ideas (obviously). Good idea example: the Scintilla imbalance with selection line delete (took me a bit of time to understand, though).

it's due to you that I learned a tiny bit of scripting here.

Well, this makes me feel good. :-) I do think scripting fills a void. Everyone wants their great idea for Notepad++ to be implemented natively, but I certainly don't agree with that, and scripting can be perfect for that situation. So I like writing and sharing scripts and helping others with them. I think I've probably written more scripts than anyone else, although @Ekopalypse is the script-master, writing way more advanced scripts than I could ever hope to.

OK, ignoring from now on...

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

No branches or pull requests

3 participants