Added conditional_before_commands #51
Conversation
Signed-off-by: Niklas Spielbauer <[email protected]>
nspielbau
force-pushed
the
conditional_before_commands
branch
from
de4d9b3 to
4543df7
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #51 +/- ##
==========================================
+ Coverage 89.24% 89.65% +0.41%
==========================================
Files 6 6
Lines 251 261 +10
==========================================
+ Hits 224 234 +10
Misses 27 27
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: Niklas Spielbauer <[email protected]>
Signed-off-by: Niklas Spielbauer <[email protected]>
Signed-off-by: Niklas Spielbauer <[email protected]>
Signed-off-by: Niklas Spielbauer <[email protected]>
|
I tested this locally with the new example_session layout and it seemed to work, the last commit was to fix an issue that is currently not caught by the test coverage tho, but I am not sure why it is not as the session was not created correctly... |
|
|
||
| if "default_window" in common and common["default_window"]: | ||
| self._default_window = common["default_window"] | ||
|
|
||
| def _parse_before_commands(self, commands): |
There was a problem hiding this comment.
Why is parsing before_commands different to commands? Maybe we should unify the concepts? In the end, both should be the same, just parsed from a different section of the config. This would obviously mean that commands would also receive an option to make them conditional, but that's not necessarily a bad thing,
There was a problem hiding this comment.
I agree, we could also extend this to the normal commands but I'll need to have a closer look at that
Sorry for all the editing I got confused :D
There was a problem hiding this comment.
I had a quick look and the command parsing is currently heavily context dependent. In a window with no Splits the directory is created explixitly, otherwise the split data is used.
Either way the directory is parsed in the split run command.
I think it would make sense to have a unfied parsing during the window __init__ either on window or split data and change the Split.run method to accept the command list directly.
This is a larger rework tho so I am not sure if we should create a new PR for that (that maybe gets merged before this one)
Co-authored-by: Felix Exner (fexner) <[email protected]>
Signed-off-by: Niklas Spielbauer <[email protected]>
|
Draft until #53 is merged |
Resolves #48