-
-
Notifications
You must be signed in to change notification settings - Fork 74
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
ANSI code being printed to terminal when using alacritty #46
Comments
Interestingly I'm not able to reproduce this. Does this happen when only one task is defined? I followed your exact steps and everything worked as expected. I'm on a MacOS. |
Well, colour me puzzled... I cannot reproduce this issue with nano, emacs or vim, either. However, the issue is still present when setting Note: |
I use neovim on my machine, and I don't have any issue. Can you run |
|
Can you run My guess is that after editing your task, something weird is going on and taskwarrior-tui is not able to import the exported output of taskwarrior. It is so surprising that it happens only with neovim. If you are able to build from source and add some Lines 810 to 844 in fa9095a
Unfortunately, since I'm not able to reproduce locally, I'm not sure how to help debug this issue. |
Any luck? |
Sorry, I have been busy and couldn't get around to this until now. Unfortunately, the output of I am happy to apply any patches and compile from source, but I am not sure as to where you would like me to put those debug statements. Could you give me a patch instead? |
So, I tried adding the
Unfortunately, I am not familiar with rust and I have no clue how to work around this... Update: Never mind... I found the documentation of the pub fn export_tasks(&mut self) -> Result<(), Box<dyn Error>> {
let mut task = Command::new("task");
task.arg("rc.json.array=on");
task.arg("rc.confirmation=off");
task.arg("export");
dbg!(&task);
let filter = if self.current_context_filter != "" {
let t = format!("{} {}", self.filter.as_str(), self.current_context_filter);
t
} else {
self.filter.as_str().into()
};
dbg!(&filter);
match shlex::split(&filter) {
Some(cmd) => {
dbg!(&cmd);
for s in cmd {
task.arg(&s);
}
}
None => {
task.arg("");
}
}
let output = task.output()?;
dbg!(&output);
let data = String::from_utf8_lossy(&output.stdout);
dbg!(&data);
let error = String::from_utf8_lossy(&output.stderr);
dbg!(&error);
if !error.contains("The expression could not be evaluated.") {
let imported = import(data.as_bytes())?;
*(self.tasks.lock().unwrap()) = imported;
self.tasks.lock().unwrap().sort_by(cmp);
}
Ok(())
} This is the
|
Thanks that is super helpful! Would it be possible for you to send me your |
This is basically the problem. |
The
Not a clue. The filter line appears only as |
I am afraid this might have something to do with |
It is not related with |
Can you share the error message after it crashes? |
The error message is:
in my case after closing the editor it appears |
Can you run |
This is the output from
|
I imported all your tasks and was able to edit them without any issues. Can you tell me which task causes a crash? |
I'm not. It appears by itself as soon as the editor exits. Also, for some reason the issue appears harder to reproduce under X11 (xmonad), while under Wayland (sway) it is consistent. |
Oh I see. It's automatically entering It must have something to do with these lines: Lines 1369 to 1371 in 5e469b8
I'm not having this issue in my environments though. Are you able to hit backspace immediately after hitting Also, if you are interested in debugging further, I would play around with the sleep timings in Line 81 in 5e469b8
There's only 4 instances of |
Yes, if I'm quick enough I am able to delete the
The only instance that seems to have any effect on the issue is the first one, at line 81: increasing the time argument delays the appearance of the |
But it always appears regardless of the size of the delay? I might have to share this question with some neovim devs to see if they might be able to share insight as to what is going on. It could very well be a neovim related issue. |
The largest value I tried is 10s, i.e. |
Can you run the following rust file (I named it
And after you edit the task and save it, see if it still prints You can run it using the following:
If you use |
I'm not sure what's going on here... the
P.S.: It appears that the crate is called Edit: Never mind... I copied your snippet as-is, but apparently |
I've tried everything I could think of and I'm not sure how to proceed on this. With alacritty and |
I've made a Minimal Working Example (MWE) of this bug. I'm able to get similar behavior with vi, vim and neovim but not with nano. Here's a gif of it in action. This is troubling because in the current implementation I'm interpreting those to be user entered key strokes, which means the ANSI codes that are being printed may cause arbitrary changes to the users task list. I'm not able to reproduce this if I remove the "threading" related pieces in my code, everything works as intended. I don't understand why this is the case, but that is good news in a way, since it means this can potentially be solved with fixes on my end (I think). I wanted to warn you about this. I don't want this program to cause intended issues with your taskwarrior setup. Maybe avoid using the The code to reproduce the issue is below. I'll post on the crossterm repo to see if they can suggest something. use crossterm::{
event,
terminal::{disable_raw_mode, enable_raw_mode},
};
use std::error::Error;
use std::process::Command;
use std::result::Result;
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::Arc;
use std::{sync::mpsc, thread, time::Duration, time::Instant};
#[derive(Debug, Clone, Copy)]
pub enum Key {
Char(char),
Null,
}
#[derive(Debug, Clone, Copy)]
pub struct EventConfig {
pub tick_rate: Duration,
}
#[derive(Debug, Clone, Copy)]
pub enum Event<I> {
Input(I),
Tick,
}
pub struct Events {
pub rx: mpsc::Receiver<Event<Key>>,
pub tx: mpsc::Sender<Event<Key>>,
pub pause_stdin: Arc<AtomicBool>,
}
impl Events {
#[cfg(feature = "crossterm")]
pub fn with_config(config: EventConfig) -> Events {
use crossterm::event::{KeyCode::*};
let (tx, rx) = mpsc::channel();
let pause_stdin = Arc::new(AtomicBool::new(false));
let tick_rate = config.tick_rate;
let _input_handle = {
let tx = tx.clone();
let pause_stdin = pause_stdin.clone();
thread::spawn(move || {
let mut last_tick = Instant::now();
loop {
let timeout = tick_rate
.checked_sub(last_tick.elapsed())
.unwrap_or_else(|| Duration::from_secs(0));
if event::poll(timeout).unwrap() {
if !pause_stdin.load(Ordering::SeqCst) {
if let event::Event::Key(key) = event::read().unwrap() {
let key = match key.code {
Char(c) => Key::Char(c),
_ => Key::Null,
};
tx.send(Event::Input(key)).unwrap();
}
}
}
if last_tick.elapsed() >= tick_rate && tx.send(Event::Tick).is_ok() {
last_tick = Instant::now();
}
}
})
};
Events { rx, tx, pause_stdin }
}
/// Attempts to read an event.
/// This function will block the current thread.
pub fn next(&self) -> Result<Event<Key>, mpsc::RecvError> {
self.rx.recv()
}
pub fn pause_event_loop(&self) {
self.pause_stdin.swap(true, Ordering::SeqCst);
}
pub fn resume_event_loop(&self) {
self.pause_stdin.swap(false, Ordering::SeqCst);
}
pub fn pause_key_capture(&self) {
self.pause_event_loop();
disable_raw_mode().unwrap();
}
pub fn resume_key_capture(&self) {
enable_raw_mode().unwrap();
self.resume_event_loop();
}
}
fn main() -> Result<(), Box<dyn Error>> {
// Terminal initialization
enable_raw_mode().unwrap();
let mut i = 0;
// Setup event handlers
let events = Events::with_config(EventConfig {
tick_rate: Duration::from_millis(1000),
});
loop {
match events.next()? {
Event::Input(input) => {
match input {
Key::Char('q') => break,
Key::Char('e') => {
events.pause_key_capture();
let r = Command::new("vim").spawn();
match r {
Ok(child) => {
let output = child.wait_with_output();
match output {
Ok(output) => {
if !output.status.success() {
Err(format!(
"process status was not success. {}{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr),
))
} else {
String::from_utf8_lossy(&output.stdout);
String::from_utf8_lossy(&output.stderr);
Ok(())
}
}
Err(err) => Err(format!("Failed to get output of process: {}", err)),
}
}
Err(err) => Err(format!("Failed to spawn process: {}", err)),
}?;
events.resume_key_capture();
},
Key::Char(c) => {
disable_raw_mode().unwrap();
print!("{}", c);
enable_raw_mode().unwrap();
},
_ => (),
};
}
Event::Tick => {
disable_raw_mode().unwrap();
println!();
println!("Tick: {}", i);
enable_raw_mode().unwrap();
i += 1;
}
};
}
disable_raw_mode().unwrap();
Ok(())
} |
After lots of experimenting and debugging I think I've fixed this. I'm no longer able to get ansi codes being printed to |
You can try it out in this release: https://github.com/kdheepak/taskwarrior-tui/releases/tag/v0.10.4 |
Thanks for the hard work you've put into this. I'll try out the new changes as soon as I have some time today or tomorrow. My guess as for why you don't see those keycodes after running |
I tried the latest commit (0ae677f) and it was much better, although I cannot say that the issue is completely resolved: in 3 out of 40 tests the issue was still present, i.e. the string |
Thanks for trying it! Can you change these lines: Lines 90 to 92 in 0ae677f
to the following: let timeout = Duration::from_millis(10)
.checked_sub(last_tick.elapsed())
.unwrap_or_else(|| Duration::from_millis(0)); and try again? If I set the timeout too low (i.e. If using |
Indeed, setting the timeout to 0ms drove the CPU usage way too high. However, setting it to 5ms seemed to solve the issue (out of 100 tests) and also kept the CPU usage to reasonable levels. Thanks again! let timeout = Duration::from_millis(10)
.checked_sub(last_tick.elapsed())
.unwrap_or_else(|| Duration::from_millis(5)); |
I think I can change it to the above timings. The CPU usage seems reasonable. Thanks for testing so many times! |
Did you test a 100 times or so manually? Or did you test programmatically? It would be good to add such a test into the test suite so there isn't a regression in the future. |
Unfortunately I did it manually. I assume it would be possible to set up an automated "puppeted" test, but that would be pretty fragile and I wouldn't recommend keeping it long term for regression testing. It should be possible to write a dummy program to use in place of an editor that changes the terminal colours (assuming that's enough to trigger a colour reset via the OSC 11 code) and immediately exits. I still wouldn't know how to check that taskwarrior-tui isn't reading part of the OSC code as input — mainly because I don't know the codebase (nor Rust, for that matter), but I presume it could be relatively straightforward to do in a unit test. |
wow thanks for being so dedicated :) And thanks for the input, I'll look into how to set up a test for this. |
I made a new release that has the fix for this issue: https://github.com/kdheepak/taskwarrior-tui/releases/tag/v0.11.0. |
I re-did the event handler using And now there's no sleeps and delays any more like I had before to avoid the ANSI codes being printed. I didn't test on linux but in my testing on my mac using alacritty and iterm2 I never had ANSI codes being printed to the terminal even once. Hopefully there's no regression here but if you do notice anything please comment here / open a new issue! |
You can try it out in this release: https://github.com/kdheepak/taskwarrior-tui/releases/tag/v0.13.0. |
In edit mode
taskwarrior-tui
crashes after exiting the editor, whether there are changes or not.Steps to reproduce the behaviour:
taskrc
and database:taskwarrior-tui
:Select the test task and open the editor with
e
Quit the editor
Output:
Note:
Setting
RUST_BACKTRACE=1
does not produce any additional output.Environment:
Operating System: Arch Linux
Installation:
taskwarrior-tui
package from the AURtaskwarrior-tui version: 0.9.3
task version: 2.5.1
The text was updated successfully, but these errors were encountered: