-
Notifications
You must be signed in to change notification settings - Fork 279
Add OSC 7 escape sequence support for tracking current working directory #611
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
base: master
Are you sure you want to change the base?
Conversation
OSC 7 is a standard escape sequence that allows shells to report their current working directory to the terminal emulator.
| /** | ||
| * Sets the initial working directory. | ||
| */ | ||
| void setInitialWorkingDirectory(const QString &dir); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| void Pty::setInitialWorkingDirectory(const QString &dir) | ||
| { | ||
| QString pwd = dir; | ||
|
|
||
| // remove trailing slash in the path when appropriate | ||
| // example: /usr/share/icons/ ==> /usr/share/icons | ||
| if (pwd.length() > 1 && pwd.endsWith(QLatin1Char('/'))) { | ||
| pwd.chop(1); | ||
| } | ||
|
|
||
| setWorkingDirectory(pwd); | ||
|
|
||
| // setting PWD to "." will cause problem for bash & zsh | ||
| if (pwd != QLatin1String(".")) { | ||
| setEnv(QStringLiteral("PWD"), pwd); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| connect(m_impl->m_session, SIGNAL(resizeRequest(QSize)), this, SLOT(setSize(QSize))); | ||
| connect(m_impl->m_session, SIGNAL(finished()), this, SLOT(sessionFinished())); | ||
| connect(m_impl->m_session, &Session::titleChanged, this, &QTermWidget::titleChanged); | ||
| connect(m_impl->m_session, &Session::currentDirectoryChanged, this, &QTermWidget::currentDirectoryChanged); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| #ifdef Q_OS_LINUX | ||
| // Christian Surlykke: On linux we could look at /proc/<pid>/cwd which should be a link to current | ||
| // working directory (<pid>: process id of the shell). I don't know about BSD. | ||
| // Maybe we could just offer it when running linux, for a start. | ||
| QDir d(QString::fromLatin1("/proc/%1/cwd").arg(getShellPID())); | ||
| if (!d.exists()) | ||
| { | ||
| qDebug() << "Cannot find" << d.dirName(); | ||
| goto fallback; | ||
| } | ||
| return d.canonicalPath(); | ||
| #endif | ||
|
|
||
| fallback: | ||
| // fallback, initial WD | ||
| return m_impl->m_session->initialWorkingDirectory(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved without modification to Session.cpp.
| /** | ||
| * Emitted when the current working directory of the active session | ||
| * has changed. | ||
| */ | ||
| void currentDirectoryChanged(const QString &dir); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if (!_initialWorkingDir.isEmpty()) { | ||
| _shellProcess->setWorkingDirectory(_initialWorkingDir); | ||
| _shellProcess->setInitialWorkingDirectory(_initialWorkingDir); | ||
| } else { | ||
| _shellProcess->setWorkingDirectory(cwd); | ||
| _shellProcess->setInitialWorkingDirectory(cwd); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if (what == 7) { | ||
| _reportedWorkingUrl = QUrl::fromUserInput(caption); | ||
| emit currentDirectoryChanged(currentWorkingDirectory()); | ||
| modified = true; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| /** | ||
| * Returns the current directory of the foreground process in the | ||
| * session. | ||
| */ | ||
| QString currentWorkingDirectory(); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| /** | ||
| * Emitted when the current working directory of this session changes. | ||
| * | ||
| * @param dir The new current working directory of the session. | ||
| */ | ||
| void currentDirectoryChanged(const QString &dir); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| int _sessionId; | ||
|
|
||
| QString _initialWorkingDir; | ||
| QUrl _reportedWorkingUrl; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
Thanks a lot for the PR! It should be checked thoroughly for making sure that it won't cause a regression anywhere. Also, there may be a more direct way of doing it. Sadly, I may not have the required time soon. So, unless another dev reviews it, please be patient. |
|
The code is taken verbatim from Konsole (references linked above) and has been stable for many years. In addition to that, I have been using it as my daily driver with no issues. There is not a more direct way of doing this. I know this because I have implemented this previously in Ghostty as well. The Konsole approach (directly copied into this PR) is standard among all modern terminal emulators. The shell keeps track of changes to the current working directory. OSC 7 codes are needed for the shell to inform the terminal emulator of directory changes. (Ghostty goes one step further than Konsole and actually injects OSC 7 code into the shell's startup sequence, a bit too much magic for me.) The terminal can then parse the OSC 7 code and know the current working directory reliably. The only remaining step is to set |
Konsole has bugs, and as I mentioned elsewhere, the codes of konsole and qterminal have diverged. |
|
All software has bugs, but code that has been shipping in production for years without any major issues being reported is less likely to have them than code that has been freshly written. This PR consists of the former rather than the latter. While Konsole has diverged plenty in other areas, in this particular area it has diverged very little. As for my patience, mentioned in a previous comment, I serve on the governing board of a large open source project, as well as an elected role in the Linux Foundation, so I am quite familiar with the time scale of open source software development. Some changes take days, other takes weeks, while yet others take months or years, but ultimately working code that solves problems prevails. |
I asked for it because I should deal with lots of codes in several apps (during the last hour, I was working on 5 different apps), and understandably I can do it only in my free time. We lack enough manpower, and I got very happy when I saw that you attended to those 2 reports. Please keep up the good work. |
|
That comes with the territory of open source projects maintained by volunteers, with which (as I mentioned) I am more than familiar. Take your time. To summarize the flow of things (in virtually all modern terminal emulators):
Hope this helps! |
|
The code seems good and works with But with this patch, when I open QTerminal inside the symlink folder with pcmanfm-qt (→ #606 (comment)), it shows the target folder instead. Konsole has the same problem but XTerm doesn't — and, of course, QTerminal didn't either without the patch. So, I think Konsole's problem is transferred to qtermwidget now. I don't think the issue can be in libfm-qt but will check its code again later. |
Fixes #606. Tested by reproducing the issue described there and verifying it no longer occurs after this PR.