Sometimes it's the tiny details that get ridiculously complicated.
Sometimes such a feature will just seem like it's only logical that it be there, and it shouldn't be that complicated, and you add hack after hack to make it work, without re-evaluating whether it's really so simple after all and whether it's worth all the hacks.
Such a feature is the ability of the same track in different views to have different widths. It makes sense, right? Seems so simple.
Not really. It means the track width has to go in the view, not the block. A view is one window on a block. The block has tracks, but if views are to have different widths for the same track, the view also needs a parallel list of track widths for per-view track data. There's various bits of code to try to maintain that invariant, and behave nicely if somehow it was broken and they are different lengths. A separate verification step checks that, among other state invariants, but now it also needs to be able to modify the state in case it wanted to fix up the broken invariant.
And then what width do tracks have in a new view? So Blocks now also have track widths, for new view defaults. Ok, a little confusing, but not too bad.
Now there's this other little thing... there are actually two types of Track, a Block.Track and a Block.DisplayTrack. A normal Track supports high level notions like Collapse and Mute and whatnot, but the UI level knows nothing of these things. A Track with Collapse set turns into a track remove and then insert of a kind of tiny degenerate track called a Divider. So if you want the real visible width, you have to look at the DisplayTrack. In the Block, not the View. And then when it gets expanded again it needs its old width... from the View! So the function to convert Block.Track -> Block.DisplayTrack... whoops, needs an extra View parameter. But it has to return that width separately, not in the DisplayTrack... because DisplayTrack is Block-level, not View-level, remember?
DisplayTrack updates are used to tell the UI to change block-level track settings, but width can't go there of course, it needs to have its own view-level track settings update.
Here's another thing, explained from the comment on the function that does the hackery:
Of course these functions weren't originally there, but were written after seeing some odd behaviour, debugging until I barely understood the situation, and how to hack in workarounds so the odd behaviour would go away.
Now here's another one: loading a new block has messed up track widths. It turns out the problem is that creating a new view has to default the track widths properly, from the View track widths if they are set, from the Block otherwise.
So, I'm getting rid of per-view track width.
Now, laid out like this, it's pretty obvious that either I'm organizing this data totally wrongly, or per-view track width is simply not worth it. It's a stupid little almost useless feature with a stupid amount of code and debugging to support it. But at the time, it was always one more little hack to support this little feature that seemed logical, and like it gave a more flexible UI. If my code couldn't support the obvious behaviour, then my code was broken, not my expectations for the behaviour. And a lot of the complexity was that it introduced a whole new possibility of per-view track data. I already had per-block track data (mute, collapse, etc.) and per-view block data (selection, scroll, zoom, etc.), so per-view track data seemed logical enough. The problem was that it never wound up being used for anything other than the stupid feature and it messed up the nice one dimensional containment hierarchy of track -> block -> view. How much time did it take to implement and debug that? I don't know, but it sure was gratifying to remove it.
Also, I have sort of a philosophy that I want to be minimal in the sense of having few types, but maximal in the flexibility of how they can be combined. So I have only Views, Blocks, Tracks, and Rulers. But any number of Views can be open on one Block, the same Track can appear in any number of Blocks, there's no restriction of the placement or ordering of Rulers and Tracks (and Dividers), etc. The c++ level doesn't understand any of this, it's totally flat, so haskell has to explicitly propagate a block change to all its views.
"Maximal" flexibility is a dangerous concept.
Go for "reasonably" flexible instead.
Post scriptum: after all the dust settled, the change got rid of about 80 lines. And about 20 of those were comments. But the comments were only huge because they were documenting around 20 lines of really icky stuff. So it's a win!
Sometimes such a feature will just seem like it's only logical that it be there, and it shouldn't be that complicated, and you add hack after hack to make it work, without re-evaluating whether it's really so simple after all and whether it's worth all the hacks.
Such a feature is the ability of the same track in different views to have different widths. It makes sense, right? Seems so simple.
Not really. It means the track width has to go in the view, not the block. A view is one window on a block. The block has tracks, but if views are to have different widths for the same track, the view also needs a parallel list of track widths for per-view track data. There's various bits of code to try to maintain that invariant, and behave nicely if somehow it was broken and they are different lengths. A separate verification step checks that, among other state invariants, but now it also needs to be able to modify the state in case it wanted to fix up the broken invariant.
And then what width do tracks have in a new view? So Blocks now also have track widths, for new view defaults. Ok, a little confusing, but not too bad.
Now there's this other little thing... there are actually two types of Track, a Block.Track and a Block.DisplayTrack. A normal Track supports high level notions like Collapse and Mute and whatnot, but the UI level knows nothing of these things. A Track with Collapse set turns into a track remove and then insert of a kind of tiny degenerate track called a Divider. So if you want the real visible width, you have to look at the DisplayTrack. In the Block, not the View. And then when it gets expanded again it needs its old width... from the View! So the function to convert Block.Track -> Block.DisplayTrack... whoops, needs an extra View parameter. But it has to return that width separately, not in the DisplayTrack... because DisplayTrack is Block-level, not View-level, remember?
DisplayTrack updates are used to tell the UI to change block-level track settings, but width can't go there of course, it needs to have its own view-level track settings update.
Here's another thing, explained from the comment on the function that does the hackery:
This is a nasty little case that falls out of how I'm doing diffs: First the view diff runs, which detects changed track widths. Then the block diff runs, which detects changed tracks. Replaced tracks (remove old track, insert new one with default width, which as a new track should get the default width in all views) are not distinguishable from a merely altered track (which can look like remove old track, insert new one with the same width, and should keep its width in each view). So what I do is assume that if there's an InsertTrack and a corresponding TrackView in view_tracks of the new state, it should get the width given in the view.
But wait! There's more! If it's a collapsed track then the track_view_width is not the visible width, so don't emit TrackWidth updates in that case.Later on we have this other fun comment:
The track view info (widths) is in the View, while the track data itself (Tracklikes) is in the Block. Since one track may have been added or deleted while another's width was changed, I have to run 'Seq.indexed_pairs' here with the Blocks' Tracklikes to pair up the the same Tracklikes before comparing their widths. 'i' will be the TrackNum index for the tracks pre insertion/deletion, which is correct since the view is diffed and its Updates run before the Block updates. This also means it actually matters that updates are run in order. This is a lot of subtlety just to detect width changes!I don't often have 9 line comments to explain code, and when you see two in one file you know something has gone seriously wrong.
Of course these functions weren't originally there, but were written after seeing some odd behaviour, debugging until I barely understood the situation, and how to hack in workarounds so the odd behaviour would go away.
Now here's another one: loading a new block has messed up track widths. It turns out the problem is that creating a new view has to default the track widths properly, from the View track widths if they are set, from the Block otherwise.
So, I'm getting rid of per-view track width.
Now, laid out like this, it's pretty obvious that either I'm organizing this data totally wrongly, or per-view track width is simply not worth it. It's a stupid little almost useless feature with a stupid amount of code and debugging to support it. But at the time, it was always one more little hack to support this little feature that seemed logical, and like it gave a more flexible UI. If my code couldn't support the obvious behaviour, then my code was broken, not my expectations for the behaviour. And a lot of the complexity was that it introduced a whole new possibility of per-view track data. I already had per-block track data (mute, collapse, etc.) and per-view block data (selection, scroll, zoom, etc.), so per-view track data seemed logical enough. The problem was that it never wound up being used for anything other than the stupid feature and it messed up the nice one dimensional containment hierarchy of track -> block -> view. How much time did it take to implement and debug that? I don't know, but it sure was gratifying to remove it.
Also, I have sort of a philosophy that I want to be minimal in the sense of having few types, but maximal in the flexibility of how they can be combined. So I have only Views, Blocks, Tracks, and Rulers. But any number of Views can be open on one Block, the same Track can appear in any number of Blocks, there's no restriction of the placement or ordering of Rulers and Tracks (and Dividers), etc. The c++ level doesn't understand any of this, it's totally flat, so haskell has to explicitly propagate a block change to all its views.
"Maximal" flexibility is a dangerous concept.
Go for "reasonably" flexible instead.
Post scriptum: after all the dust settled, the change got rid of about 80 lines. And about 20 of those were comments. But the comments were only huge because they were documenting around 20 lines of really icky stuff. So it's a win!
No comments:
Post a Comment