Skip to content

Comments

Comply with cargo fmt style guidelines.#15

Open
maaku wants to merge 1 commit intomrmekon:masterfrom
maaku:cargo-fmt
Open

Comply with cargo fmt style guidelines.#15
maaku wants to merge 1 commit intomrmekon:masterfrom
maaku:cargo-fmt

Conversation

@maaku
Copy link
Contributor

@maaku maaku commented Mar 23, 2020

I don't know if this is a welcome contribution or not, but this repo doesn't seem to be applying cargo fmt style guidelines. This patch fixes up the source code to be cargo-formatted.

(Like many rust developers, my editors are configured to auto-format source files. I have other contributions I'd like to make, and switching to the standard style formatting first would streamline the process for me. But if this is not a desired change, I understand.)

@mrmekon
Copy link
Owner

mrmekon commented May 18, 2020

@maaku What are the other contributions you are considering? I am not a big fan of auto-formatting, even though most of the changes are improvements. I don't plan on merging this in isolation, but might be willing to if it leads to other improvements.

My main thing against auto-formatting is it converts long horizontals lines into long vertical blocks. Sometimes that's good... but sometimes I use long horizontals on purpose to make it easier to scan quickly. In this patch, I consider the stubbed default implementation to be much uglier post-formatting. A bunch of easily scannable single lines are converted to 3-4 line blocks that take a ton of vertical space.

@maaku
Copy link
Contributor Author

maaku commented May 19, 2020

The main thing I was editing the crate to do is the leave the original application in a trampoline operation open, and setup pipes for stdin/stdout/stderr to or from the child process. That way when launching an application from the command line, for example, the output (logs, etc.) still shows up in the terminal.

Though if you have a better way of achieving this goal, I'm all ears. I got about halfway done with setting this up on my own repo which is auto-formatted, before I got distracted by other tasks.

@mrmekon
Copy link
Owner

mrmekon commented Jul 9, 2020

@maaku Sorry for the delayed responses. I'm in full support of the communication pipes, sounds like a great idea. If you get a clean solution for that worked up, I'll happily merge it in, along with the code formatting. I'll just leave this open for now, but won't merge until there's something ready to go on top of it.

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.

2 participants