Skip to content

Conversation

cipriancraciun
Copy link
Contributor

I have made three (quite minor) changes, that are most helpful in scripted scenarios, where dtach is used behind the scenes. Namely:

  • add support for -- argument termination marker (just like other Linux tools), which allows one to have executables starting with -; another side-effect of this patch is that in case dtach is used in a script where we can't trust the user input, now the user is not able to inject new options that are interpreted by dtach; (for example it is used like dtach -c socket -- ${@};

  • add support for -q option which, like other Linux tools makes dtach exit quietly if everything is OK; basically it doesn't print anymore [EOS ...]; in conjunction with the other patches which introduce the -R option, it makes dtach transparent when used in scripts;

  • add support for -R (the symmetric argument for -r), which allows the user how the terminal is cleared when the command exits or is detached; the two options are move (the current behavior) and none, which just prints a new line; the second variant allows one to use dtach in a loop for short commands, and still be able to read their output without having to scroll; (one use-case is for example in a Jenkins job;)

If these patches are accepted I can also provide the necessary man updates. (For now I have only added the embedded help documentation.)

@crigler
Copy link
Owner

crigler commented Jun 18, 2025

I cherry-picked the commit to add the -- argument termination marker support.

The other commits feel messy to me.

@cipriancraciun
Copy link
Contributor Author

The other commits feel messy to me.

Is there something I could do to fix the other ones?

Since opening this pull request I've relied heavily on the -q and -R in scripts to make dtach fit nicely with other stderr messages (outside of dtach).

@crigler
Copy link
Owner

crigler commented Jun 22, 2025

The first replace EOS with clear_csi_data() commit is incomplete and does not build successfully, and a later commit changes "\33[H\33[J" to "\033c" without explanation. The commit to reset graphic attributes also has no explanation. Some of the commits also do not match dtach's coding style, and use C++ comments which could break the build with some compilers.

Should the terminal always be completely reset before attaching and should graphic attributes always be reset on exit? That might be an unexpected behavior change, for example dtach is used in environments like proxmox to wrap VM consoles where minimal processing may be desired.

The git repository will exist long after github is gone, and someone should not have to look at a PR/issue on github to understand the why of a commit.

https://github.blog/developer-skills/github/write-better-commits-build-better-projects/ and https://cbea.ms/git-commit/ explain why and how to write better commits and pull requests.

I think there is also disagreement on what to replace dtach's assumptions with. PR #5 uses system("tput init 2>/dev/null") instead. We may all in the community have to decide what to do about this before making any code changes, and these old assumptions have been in place for a very long time now.

@crigler
Copy link
Owner

crigler commented Jun 23, 2025

Looking at some old issues such as #14 and #15 reminded me that some programs do change the terminal state upon startup, so always completely resetting the terminal when attaching would break things for some users.

These commits are doing a lot more than the "quite minor" changes described in the initial pull request comment.

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