Feature: external editor support #123#177
Feature: external editor support #123#177GachiLord wants to merge 6 commits intoJulien-cpsn:mainfrom
Conversation
|
Hello, Very good pr thanks!!! I reviewed most of it on my phone (which is a pretty good sign). My only criticism before testing it would be that every event needs its function, so you should move the business logic in an appropriate file. I can do it myself, no problem about that. That'll fix the "Editing body is hard" problem I guess (I took it personally 😢) Have a great day too!! |
|
It would be nice if you created a function for handling those events by yourself. Thanks. Also there is a common wisdom that everyone tries to create their own vim just to see it's not as neet as a real one :) |
|
Is this PR going to get merged? Looks like its been outstanding for a couple of months now. |
|
Hi! As said in others PR/issues/discord, I will only merge quality code and deeply reviewed PRs, and it takes much time. I really want to give full attention to this work. You're the next on the TODO list tbh Have a great day |
| use crate::tui::utils::stateful::text_input::TextInput; | ||
|
|
||
|
|
||
| pub fn run(terminal: &mut Terminal<impl Backend>, default_text: &str, file_extension: &str) -> Result<String> { |
| tracing-log = "=0.2.0" | ||
| ## Opentracing middleware implementation for reqwest-middleware | ||
| reqwest-tracing = "=0.5.8" No newline at end of file | ||
| reqwest-tracing = "=0.5.8" |
| yank_response_part = "y" # Used to yank the current result tab (e.g. body, headers, cookies) | ||
|
|
||
| result_next_tab = "Shift-BackTab" # Will use param_next_tab depending on the selected view No newline at end of file | ||
| result_next_tab = "Shift-BackTab" # Will use param_next_tab depending on the selected view |
| yank_response_part = "y" # Used to yank the current result tab (e.g. body, headers, cookies) | ||
|
|
||
| result_next_tab = "Shift-BackTab" # Will use param_next_tab depending on the selected view No newline at end of file | ||
| result_next_tab = "Shift-BackTab" # Will use param_next_tab depending on the selected view |
| yank_response_part = "Shift-Y" # Used to yank the current result tab (e.g. body, headers, cookies) | ||
|
|
||
| result_next_tab = "Ctrl-t" # Will use param_next_tab depending on the selected view No newline at end of file | ||
| result_next_tab = "Ctrl-t" # Will use param_next_tab depending on the selected view |
| KeyCombination { codes: One(KeyCode::Char(char)), .. } => self.body_text_area.insert_char(char), | ||
| _ => {} | ||
| }, | ||
| EditRequestBodySystemEditor(_) => { |
There was a problem hiding this comment.
Should be in a separate function
| let request = local_selected_request.read(); | ||
| let request = request.get_http_request().unwrap(); | ||
|
|
||
| match request.body { |
There was a problem hiding this comment.
Should create a method for ContentType called to_file_extension
| pub fn tui_request_body_file_extension(&self) -> Option<&'static str> { | ||
| let selected_request_index = &self.collections_tree.selected.unwrap(); | ||
| let local_selected_request = self.get_request_as_local_from_indexes(selected_request_index); | ||
| let request = local_selected_request.read(); |
There was a problem hiding this comment.
Should be placed in a sub-scope
| let selected_request_index = &self.collections_tree.selected.unwrap(); | ||
| let local_selected_request = self.get_request_as_local_from_indexes(selected_request_index); | ||
| let request = local_selected_request.read(); | ||
| let request = request.get_http_request().unwrap(); |
There was a problem hiding this comment.
Should be renamed http_request
| self.reset_cursor(); | ||
| } | ||
|
|
||
| pub fn set_input(&mut self, text: String) { |
There was a problem hiding this comment.
Should be renamed set_input_content
| Documentation(EventKeyBinding::new(vec![key_bindings.generic.display_help], "Display help", Some("Help"))), | ||
|
|
||
| EditUrl(EventKeyBinding::new(vec![key_bindings.request_selected.change_url], "Edit URL", Some("URL"))), | ||
| EditUrlSystemEditor(EventKeyBinding::new(vec![key_bindings.request_selected.alt_change_url], "Edit URL via system editor", Some("URL"))), |
| ], | ||
| RequestParamsTabs::Body => vec![ | ||
| EditRequestBody(EventKeyBinding::new(vec![key_bindings.generic.list_and_table_actions.edit_element], "Edit body", None)), | ||
| EditRequestBodySystemEditor(EventKeyBinding::new(vec![key_bindings.generic.list_and_table_actions.alt_edit_element], "Edit body via system editor", None)), |
| ], | ||
| RequestParamsTabs::Scripts => vec![ | ||
| EditRequestScript(EventKeyBinding::new(vec![key_bindings.generic.list_and_table_actions.edit_element], "Edit request script", Some("Edit"))), | ||
| EditRequestScriptSystemEditor(EventKeyBinding::new(vec![key_bindings.generic.list_and_table_actions.alt_edit_element], "Edit request script via system editor", None)), |
|
The review above is just for me to remember what's to change!! |
|
Hi, after using this for a while I noticed that provided VIM keybindings conflict with IMHO there has to be a conflict detection mechanism to prevent this kind of situations |
|
Hello @GachiLord Sorry I think that my previous comment got deleted. I was asking if the system editor could be a config option rather than a keybinding. Like you put What do you think of this? I was just wondering of this being more practical, but I am not a user of this feature I think your PR is great but I need to work on it to make it better Btw the reviews are just for me to remember what needs to change Have a great day |
|
Yeah, your idea sounds great, though it's a lot more code to change. Guess I can rework the PR or create new one to implement this feature. What do you think? |
|
@GachiLord you've done enough work, I will work on it If you have any requests or comments to add, go on! |
|
I think there is nothing to add. Looking forward to new release! |
|
Added in 4ad3277 |
What's new
This PR adds ability to modify the following request properties via system's editor:
The editor is invoked via alternative keybinding, so the original way of editing still works and users can choose what they prefer. The keybindings are included in this PR, but I haven't thought on the usability much so feel free to change them.
Mac and default keybindings:
Vim keybindings:
What's changed
Testing
It works on alacritty 0.15.1(openSUSE 6.16.7-1-default) and Gnome Terminal 48.1(Fedora 42) but I haven't tested it on other platforms.
Sorry for many LOC and long PR message, have a nice day!