Page MenuHomePhabricator

Option to disable cd to current tab dir.
ClosedPublic

Authored by TheIndifferent on Dec 24 2015, 7:07 AM.

Details

Summary

Adding option to disable cd to current tab dir.

Test Plan
  1. Start unpatched version of Terminology.
  2. Change some Behaviour options.
  3. Close unpatched version of Terminology.
  4. Start patched version of Terminology.
  5. cd /usr/bin
  6. Open new tab and verify that previous behaviour persists, new tab working directory is /usr/bin
  7. Go to Settings Behaviour and uncheck "Start in the same directory..." checkbox.
  8. Optn new tab and verify that new tab working directory is ~ (or any other directory from which the Terminology was launched).
  9. Restart Terminology.
  10. Go to Settings Behaviour and verify that options persisted.

Diff Detail

Repository
rTRM apps/terminology
Branch
start-in-workdir
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 934
Build 999: arc lint + arc unit
TheIndifferent retitled this revision from to Adding option to disable cd to current tab dir..
TheIndifferent updated this object.
TheIndifferent edited the test plan for this revision. (Show Details)
TheIndifferent retitled this revision from Adding option to disable cd to current tab dir. to Option to disable cd to current tab dir..Dec 24 2015, 7:14 AM
TheIndifferent edited the test plan for this revision. (Show Details)
TheIndifferent added a reviewer: Terminology.
TheIndifferent added a project: Terminology.
TheIndifferent edited subscribers, added: Terminology; removed: billiob.
billiob edited edge metadata.Dec 24 2015, 7:19 AM

I have one remark to do on that patch.
Could you change it from "Disable…" to "Enable…"? You should changed the default configuration to have it enabled by default, have a configuration upgrade in config_load().

Man you're fast, you reviewed it before I even added reviewers!

I have one remark to do on that patch.
Could you change it from "Disable…" to "Enable…"? You should changed the default configuration to have it enabled by default, have a configuration upgrade in config_load().

But that means that I will have to increase the config version to 7 (to preserve existing behaviour as default), right? I did it at first but then changed to inverted not to touch config version for such minor thing.

You can increase it, it's here for that. I did something like that in 144e0b5068aa25b7fce822a94101586f374aa236

Hm ok, will change that, that indeed looks nicer than inverted flags. I'll need couple of hours to figure out arc+phab.

take your time, no rush :)

Also, would appreciate advice on how to name the variable. changedir_to_current is the best I can think of...

TheIndifferent edited the test plan for this revision. (Show Details)
TheIndifferent edited edge metadata.

Changedir option changed to non-inverted.

Thats a nice configuration option!
I'm wondering if we should also put it inside _split_split() and/or _win_split().

@billiob, what do you say?

@godfath3r I agree.
In that case, the option configuration should be "Open new terminals in current working directory".

Wait, you got me confused, do you mean current option should be renamed and similar behaviour added to split methods, or one more option should be added for that?

Keep the code, just add the check for the option in the split code and change the line in options_behavior.c.

  • Changedir option changed to non-inverted.
  • Option to change working directory added to split methods.
billiob accepted this revision.Dec 26 2015, 3:08 AM
billiob edited edge metadata.
This revision is now accepted and ready to land.Dec 26 2015, 3:08 AM
billiob closed this revision.Dec 26 2015, 3:12 AM