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

Upgrade prettytable-rs to 0.8 #45

Closed
wants to merge 2 commits into from

Conversation

cuviper
Copy link

@cuviper cuviper commented Jun 6, 2020

I wasn't getting any color with TERM=tmux-256color, neither automatic nor forced always, so I think it didn't understand that terminal. I do get automatic color with prettytable-rs 0.8.

@cuviper
Copy link
Author

cuviper commented Sep 28, 2020

@BurntSushi ping?

@BurntSushi
Copy link
Owner

@cuviper Thanks. There appear to be a whole bunch of unrelated changes to the Cargo.lock though. Did you just run cargo update -p prettytable-rs or cargo update? I'd prefer the former.

@cuviper
Copy link
Author

cuviper commented Sep 28, 2020

Hmm, I don't remember why I wanted the broad cargo update, though at least I did that in a separate initial commit. I think that may have been to deal with indirect updates breaking 1.28, but I'll try to redo this with the minimal change.

@BurntSushi
Copy link
Owner

Updating the Rust version is okay. Latest stable is fine with me for a tool like this.

@BurntSushi
Copy link
Owner

Basically, my main thing here is that I like to do dependency updates myself so I can see what's going on. Maybe that would be quicker. Would you be okay with me doing that and closing this PR?

@cuviper
Copy link
Author

cuviper commented Sep 28, 2020

I don't mind if you want to handle this yourself. I just wanted to fix my colors. 🙂

@BurntSushi
Copy link
Owner

BurntSushi commented Sep 28, 2020

Ug. I was afraid of this happening. But it looks like this causes a update to the term dependency of prettytable-rs, and that in turn brings in a host of new dependencies. And in particular, it adds a dependency on the dirs crate, which I'm not comfortable putting in any of my dependency trees (for non-technical reasons).

I'm not sure what to do about this. Seems like the only option is to abandon prettytable-rs. I'm surprised people are still using term. (Well, looks like prettyable-rs was last updated two years ago, so that's probably another reason to abandon it.)

@BurntSushi
Copy link
Owner

I pushed an update to as many dependencies as I could to master. But I'd guess that won't fix your problem.

@cuviper
Copy link
Author

cuviper commented Sep 28, 2020

Yeah, the updated master is still uncolored for me.

I also tried prettytable-rs 0.7, which does fix my colors, but that's pretty much the same dependency story. I guess it's the update from term 0.4 to 0.5 that started recognizing my terminal.

And in particular, it adds a dependency on the dirs crate, which I'm not comfortable putting in any of my dependency trees (for non-technical reasons).

It looks like that was added in term 0.5.2, whereas 0.5.1 only depended on byteorder and winapi. Are you willing to just hold the term dependency back in your lock file?

@cuviper
Copy link
Author

cuviper commented Sep 28, 2020

It seems that was only added to avoid the deprecated std::env::home_dir -- Stebalien/term@84cfdb5, so maybe there's another dependency that could fill this need. (But I don't know your reasons for avoiding dirs.)

@BurntSushi
Copy link
Owner

Nice, works for me. Should be fixed in cargo-benchcmp 0.4.3 on crates.io.

And yes, I would have just kept to std::env::home_dir and stuck an allow(deprecated) on it.

@cuviper
Copy link
Author

cuviper commented Sep 28, 2020

Thanks!

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

Successfully merging this pull request may close these issues.

None yet

2 participants