simpler file modification indicator #2831
Conversation
|
The file modification indicator is also used in the buffer picker, but instead of |
|
@sudormrfbin - thanks for that (I hardly use the buffer picker). I made the change there. This will also likely impact #2759 as well. Depending on which PR gets merged first, I'll make the necessary changes. (edited to add: I'm using the same string that's in the statusbar, even though the current behavior is to strip the enclosing brackets.) |
|
Updated to use new statusline. I think a decision needs to be made here. Current implementation is arguably inconsistent: file modification in the statusline is Should we modify the buffer picker code to strip any brackets/parens to replicate the current behavior? This seems a bit magical. |
| flags.push(self.modified_indicator.to_string()); | ||
| } | ||
| if self.is_current { | ||
| flags.push("*"); | ||
| flags.push(self.current_indicator.to_string()); |
There was a problem hiding this comment.
Just push &self.current_indicator, ``&self.modified_indicatorto avoidto_string()`
There was a problem hiding this comment.
If we do this then we have to modify the join on line 2241 so that it gets a Vec<String> instead of a Vec<&String>. Happy to make this change since I'm not sure which approach is more idiomatic. (Also, do you recommend an into() for this or a map()?)
| modified_indicator: cx.editor.config().file_modification_indicator.clone(), | ||
| current_indicator: DEFAULT_CURRENT_BUFFER_INDICATOR.to_string(), |
There was a problem hiding this comment.
Hmm, this is going to be a bunch of allocations per document on the list...
Is customizability here really that necessary? From what I can tell vim simply always uses [+] and I can't imagine a lot of users will change this.
There was a problem hiding this comment.
Hmm, this is going to be a bunch of allocations per document on the list...
Could you explain this further? isn't it just one (few-byte) string allocation per open document? As far as I can tell it's mapped to open documents on line 2260 and only in the buffer picker, but I'm undoubtedly missing something.
Is customizability here really that necessary?
It's certainly not necessary, but it's a nice quality-of-life that I miss from my existing nvim setup that shouldn't impact too many people. (I use a Δ to indicate changed files and leave +/-/~ for git indications.)
It also gets const strings out of the middle of multiple code blocks and centralized, which I think is very important. Right now if you want to change a symbol there are multiple places you need to go to do it.
| @@ -2225,10 +2229,10 @@ fn buffer_picker(cx: &mut Context) { | |||
|
|
|||
| let mut flags = Vec::new(); | |||
| if self.is_modified { | |||
| flags.push("+"); | |||
| flags.push(self.modified_indicator.to_string()); | |||
There was a problem hiding this comment.
Note that this changes how the default looks: we used + but instead [+] will be used now
There was a problem hiding this comment.
Yes - this requires some discussion (c.f. #2831 (comment))
|
@archseer - what's your preference here? I don't really like the (current) setup where we have two ways of indicating a file has been modified (with and without brackets) but this will require a decision. (It also may have implications for bufferline.) |
Supersedes #2829 - editor instance is present in
render_view()so we can just pass editor to the otherrender_*functions there.