Today work was centered around getting the implementation of the latest patch for the write-to-disk bug finished. I can produly say no more bugs seem to have creeped in the latest test runs, and the issue can probably be considered fixed (of course, no actual proof exists at present, so no guarantees can be made on the correctness of the algorithm/approach.) Beyond that, the basics for scrolling support have been implemented and should be fairly straightforward to improve upon.
Work on the bugfix started with a patch that completely refactored the implementation in the effect_changes routine as I realized that the issues lied in contention and file handle overwriting when accessing file descriptors between tasks running in parallel. As it turns out, the implementation for FormatFile was behaving as expected, but the fact that I launched a separate task for each constant whose modified flag had been toggled meant that multiple constants sourced from the same file would be reading in from the same file descriptor and writing to the same file at once (potentially in parallel.) This then caused the end result on the file to look considerably inconsistent across runs, as each parallel execution of the tasks could yield one result or another (e.g. some constant gets to the file before some constant , and modifies it such that its deprecation attribute appears first, while constnat , itself marked for undeprecation, ends up still deprecated, alongside constant .) The solution was actually fairly straightforward; Group all constants sourced from the same file into a single โbucket.โ Initially, I implemented this in terms of a call to Itertools::batching that manipulated the iterator within a call chain spawned by the iterator itself, but then found out about Itertools::GroupingMap and realized a HashMap of file paths to constants gathered in a single contiguous container (Vec) would do me just fine. This was easily accomplished with Itertools::into_group_map. Then, for each yield path-to-constants item, I spawned a single task per file that synchronously called change_constants_in (part of the patch that also introduced FormatFileโs functionality, serving as a single entry point to constructing the type and calling FormatFile::mutate with the provided closure.) This makes it so that all modified constants sourced from the same file path are saved to disk sequentially. Whatever minor issues remain are related to formatting and are not very much noticeable, so they may be fixed next week.
Then, to update the spans of the memory-resident constants (kept as our custom type Const) we return from each of those tasks the file path with which it got spawned, and call a new routine, ConstContainer::update_spans. This method traverses the entire ConstContainer with a filtering operation to check only for constants whose source file path is equivalent to that of the passed file path (the one returned from each task) and updates the spans of all such matched symbols to the spans of same symbols in the file, after parsing it anew with syn. The reason why we can zip both the iterator of the ConstContainer with the iterator of the parsed file (which itself is a result of some more pre-processing to extract again constants from both module-leve scope and from cfg_if invocation macro bodies) is that both iterators are guaranteed to have the same โlength.โ Indeed, deprecation and undeprecation of symbols does not remove the effective amount of symbols in a given file, so parsing such symbols anew should yield the same symbols as those in memory that are known to have been sourced from that file. Of course, in matters of order, we go down the easy route (which happens to not be the most efficient) and sort both iterators by line/column span prior to zipping them.
Work on the scrolling implementation has greatly progressed today, and I genuinely believe it can be finished either tomorrow or the day after. At present, scrolling works adequately (but certainly not correctly) when in normal mode and with a filter list that contains at least 10 items. The implementation is fairly simple, and may even be further simplified in the coming patches. The running state keeps track of a new type, VisibleList, which ensures statically through its inner type, ConstrainedList, that the underlying bounds are always covering a set amount no larger than the polymorphic constant N over which ConstrainedList exists. I believe this can further simplified by only keeping track of a single lower bound, which would either be advanced or retracted as seen fit. This seems feasible as, outside its implementation details, the type is only used to fetch the value of its lower bound, such that we may skip that many constants when traversing the containers that hold them.
None at present.
Thankfully, scrolling support is still very much achievable, and the bug (which spawned more bugs) that I had been solving for the last few days has been (at last at the time of writing) solved. With this new outlook, I expect to have finished the scrolling implementation by tomorrow, and to quite possibly have ironed out whatever bugs were left of it by the day after. This should align pretty well with this weekโs goals, namely to fix the afore mentioned bug and to have scrolling support implemented. It would also be great if I could get some time to enhance the implementation of the path display in the list of filtered symbols, but weโll see.