-
Notifications
You must be signed in to change notification settings - Fork 629
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
[Dmenu] Implement a select rows for dmenu multi-select. #1807
[Dmenu] Implement a select rows for dmenu multi-select. #1807
Conversation
|
||
.PP | ||
If multi-select is enabled, pre-select rows, See \fB\fC-a\fR option for format details. | ||
If same row is specified multiple times, it state is toggled on subsequential sets. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this behavior is surprising. I prefer the box is checked if the row id appears more than once, and unchecked if the row id doesn't appear.
Is there any reason you think toggling based on odd/even number of occurence a superior behavior?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would make it so much easier to select all rows except a number of rows...
I am trying to make things easier for the user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see.
I would have suggested adding an option named -unselect-rows
, but then I realized it cannot substitute -select-rows $FOO,$BAR,$BAZ
to mean selecting $BAZ
and $FOO
except $BAR
.
So now the question is: This python-like range syntax is also used in other options as well, do these options also support this toggling behavior? If they do, it is consistent. If they don't, it means that the range syntax is lacking, and instead of adding toggling behavior, we should improve the range syntax (in other PR/discussion ofc).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This python-like range syntax is also used in other options as well, do these options also support this toggling behavior?
There is another option called -a
to make "active rows". And it does not have the toggling behavior. Which means that -a 1,2,3,2
would make 1, 2, 3 active whilst -select-rows 1,2,3,2
only checks 1 and 3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neither '-a' or '-u' document that it has the toggling behaviour. I have no idea why you think it does?
I expect that developers of script that use advanced features like this will read the documentation and can understand the difference.
Its a feature I can support for select-row, but not (easily) for the others, I can see this as a benefit for the script developer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(yes sorry, I misread and corrected my post already. It seems we did this in parallel).
I was saying about the consistency because the proposed solution ('&','!' ) or the new one now ( 'and' and 'not') is not the type of syntax we in rofi.
If this is so important we should not introduce another one.
To specify multiple items, we use ',' separator in several places.
entry, prompt {
}
window {
children: [inputbar, listview];
}
or
rofi -show drun -modes 'drun,window'
So if we list multiple ranges we should stick to ',' as list separator.
To negate we use '-' in the search entry box. But this is not ideal for this usecase.
(you can search for 'browser -chrome' to search all non chrome browser.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is not the type of syntax we in rofi.
You need to complete this sentence or I wouldn't understand.
If this is so important we should not introduce another one.
To specify multiple items, we use ',' separator in several places.
Reusing ,
wouldn't make sense because ,
is for adding items, not for intersecting set of items. In other words, ,
is an or
operator, different from an and
operator. 0:4,2:5
is 0,1,2,3,4
whereas 0:4 and 2:5
would be 2,3
.
If this is so important
It's not important for my use cases right now, neither is the toggling feature, I only brought this up because I dislike the toggling behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To negate we use '-' in the search entry box. But this is not ideal for this usecase.
(you can search for 'browser -chrome' to search all non chrome browser.)
Because negative numbers are a thing.
This gives me an idea: Many websites have a search syntax similar to GitHub issue (k1:v1 k2:v2 pattern
). So rofi could potentially introduce a similar syntax: 0:100 not:(10:90 not:40:50)
; or, if you want the not
keyword to be more easily discernible from normal text: 0:100 @not:(10:90 @not:40:50)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is not the type of syntax we in rofi.
You need to complete this sentence or I wouldn't understand.
is not the type of syntax we use in rofi.
Reusing , wouldn't make sense because , is for adding items, not for intersecting set of items. In other words, , is an or operator, different from and and operator. 0:4,2:5 is 0,1,2,3,4 whereas 0:4 and 2:5 would be 2,3.
eeuh what?? I was talking about adding items not intersecting, so ',' makes sense for having a list of ranges that should be selected. (As it is now). We use ',' to separate in a list already. How we specify what we then exclude then is not clear.
If we start describing thing in set theory.. then we should use the right syntax for that.. I would be fine with using ∪,∩, \ (union, intersection, set diff if i remember correctly) But I think we will have lost most of the developers/users by now.
Anyway I give up. If anybody wants to pick this up and create a PR , I'll reconsider.
I spend more time then I care to spend on/have for this already
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. I have created #1809 which removes the toggling behavior.
I only proposed a variety of different syntaxes to replace toggling because I dislike it.
@@ -147,6 +147,13 @@ Or any combination: '5,-3:,7:11,2,0,-9' | |||
.PP | |||
Urgent row, mark \fIX\fP as urgent. See \fB\fC-a\fR option for details. | |||
|
|||
.PP | |||
\fB\fC-select-rows\fR \fIX\fP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The term "select" can be confusing since it already used to mean highlighting the current item in the list before either confirm selection or check the box.
Then again, Rofi has already name the flag -multi-select
so it could be that the maintainers do care about this term.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is the term that is used everywhere else.. Using another term here would be confusing.
Multi-select is a nice to have and does not completely fit anyway.
I just cloned this branch and tested it. It work great so far. |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Issue: #1806
Your description here...