Closed faelin closed 3 years ago
You can revert back to using spaces by clicking the Spaces: 4
text in the status bar and selecting Convert indentation to tabs
:
I use this fork. It works as expected for me. I see no reason not to merge! Thanks @faelin
@rchl thank you for converting the whitespacing, apparently I completely forgot to do that after you suggested it!
While testing, I've noticed that if we have two columns and one of them is split in two rows then switching away from the column with two rows doesn't destroy the empty row pane. That seems unexpected considered the description of the option that doesn't mention any exceptions like that.
https://user-images.githubusercontent.com/153197/110373654-06843a00-8050-11eb-99f6-1d024943c71d.mov
While testing, I've noticed that if we have two columns and one of them is split in two rows then switching away from the column with two rows doesn't destroy the empty row pane. That seems unexpected considered the description of the option that doesn't mention any exceptions like that.
What you describe is a symptom of how Sublime handles its layout. When a cell is created, Origami generates a new column/row division based on shared boundaries. Sublime interprets any cells that share a column/row division as being in a contiguous group. Because of this, adjacency is only determinate for cases in which the layout contains a single cell in the group found in the direction opposite travel from the destination. To illustrate this, I have provided the following demonstration of cell grouping: (I can't figure out why this video won't embed properly, sorry)
However, I have begun considering how to better handle cell-collapsing behavior. I have illustrated my thinking process below.
With a cursor in B, the user travels left, placing the cursor in A. What cells should be culled? A naïve answer would be to specifically cull Cell B, as this is the cell we just left. However, what becomes unclear is how to expand cells to fill the space: Consider also that both the cells B and D are adjacent and in the direction opposite the travel (and both are empty), and thus could both be considered valid target cells to be destroyed, as in the third example. Unfortunately, different users are likely to have different preferences, depending on things like monitor dimensions, window layout, etc.
Because of the manner in which Sublime handles cell grouping, Origami does not always have a clear sense of adjacency. I can think of two reasonable possible to this:
In the latter case, my proposal is to have the remaining space filled by the cell in the row/column that is perpendicular to the direction of movement. In the case of the above scenario, this would result in Example 1: when moving from B to A, the remaining space is filled by the cell that is perpendicularly adjacent to the path of movement, i.e. Cell D:
In a vertical travel, the cells would fill horizontally:
Finally, cell-expansion would be prioritized top-to-bottom and left-to-right, so as to handle edge-cases of uncertainty such as this final illustration below:
I appreciate the detailed explanation and sorry for not getting back to you earlier.
Even though I have planned so, I've never managed to invest enough time into Origami to incorporate it into my workflows so I'm not super invested in it and don't have a very strong opinion on this matter either.
For me the options right now are either:
Let me know what you think and we can go either way.
Personally, I use this functionality literally every day that I work in Sublime (which is why I added the ability)!
My strong preference would be to merge in the current implementation; as it stands, even in the error-case you described, a user can still collapse the empty panes by simply traversing their cursor in the reverse of the order in which they created the pane (i.e. "retracting their steps"), so even though the auto-destroy is not perfect, it can still ultimately handle even the situation you describe.
I will continue fiddling with the code in my spare time, and can generate a new PR when I have a more comprehensive fix. In the meanwhile, users already expect to destroy panes via explicit commands, and so it seems reasonable to assume that new adopters of this feature would be able to use the "destroy pane" command if they somehow get stuck.
To be clear, I want to stress that users can still automatically destroy panes by simply traversing to the sub-divided pane (in the example layout you constructed, I would have labeled it "E" per the images above) and then traversing upward to the "parent" of the group.
This is a small change to Origami that provides a configurable option ("destroy_empty_panes") in the Origami settings (default is to ignore this behavior).
When enabled, Origami will automatically destroy panes that no longer contain any content (i.e. no open views) when traveling away from them.
The following GIFs demonstrate the new functionality:
(note: I accidentally modified whitespace, and I am not git-savvy enough to easily fix this via patch)