Refactor fsharp compute indentation #222
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Clean up tabs, indentation.
These are two modules that do tightly related things. In two separate files, they can't cleanly share variables (`indent-smie` re-defined an indent offset var also defined in `indent`). This commit merges them together and re-names the variable used for offset by the `smie` configs, but does nothing else.
This commit renames `fsharp-mode-indent` and cleans up references to it. It also scoots `fsharp-eval-phrase` next to `fsharp-eval-region` in `fsharp-mode`, where it makes a bit more sense. Tests are all currently passing.
This commit removes some old "major mode boilerplate" from `fsharp-mode-structure`: - The abbrev table is moved in to `fsharp-mode`, where it belongs. - `fsharp-safe` is _only_ used to wrap calls to `search-backwards`; the only value it provides is to swallow errors and instead return `nil`. However: this can now be much more idiomatically achieved by passing `t` as the `NOERROR` argument to `search-backward`. - XEmacs hasn't had a release in ten years. The extra region setting calls of `fsharp-keep-region-active` aren't needed -- it's just extra noise.
This allows it to do precisely the same thing, while making very sure to fulfill the guarantee of not changing whatever point and mark the user set.
`fsharp-compute-indentation` is one of the critical driving methods of this mode's indentation system. At the start of this commit, it was also a very long, very complex method. It needs better documentation and it needs to be under test. Therefore: extract method. This commit pulls a variety of method out of `fsharp-compute-indentation`, seeking to preserve basically identical functionality while capturing pieces of logic in smaller, better documented pieces. Functions are currently named after my best read of what they _do_, but that's hard to be confident about. The _next_ piece of work will be getting these functions under test, which will allow me to pin down and describe their functionality (and its limits!).
This puts `fsharp-nesting-level` under test, and updates the docstring of that function with information secured during testing.
We don't need it. Remove and update calls.
This is all notes, comments, docstrings, spacing. Preparation for further work.
Group related functions; move some `info-look` configs into `fsharp-mode.el`. This is getting to be a lot easier to navigate, look at, understand all together.
Originally, all of these functions were inside one huge save-excursion form. To my surprise, they typically need their _own_ save excursion wrapping to make sure everything is consistent, particularly under test. Also added, a great deal of notes about the function itself and how it works.
`fsharp-compute-indentation` is a huge cond, and uses a final `t` case as its default condition. This moves that condition into its own function (complete with save-excursion) so that it can be tested on its own terms.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Important: this PR is the third in a logical sequence, and contains changes from both #219 and #221, which should be reviewed first, in order. (Or, you could view all the work together, here, in a single PR, if you prefer.)
This PR is part of work documented in #218.
What's here?
This PR introduces the first set of deep structural changes to
fsharp-mode-structure. Specifically, it takes as a goal to:fsharp-compute-indentationunder test.That is: a tremendous difficulty in working with this code is that none of it is tested and most of it was lifted from
python-mode. The genesis in Python means that most names and docstrings do not accurately describe the functions or variables they're associated with. Further, prior to this work,fsharp-compute-indentationwas nearly 175 lines long.This PR works to fix this, and enable future work, but breaking
fsharp-compute-indentationinto a sequence of predicate functions and computation functions, withfsharp-compute-indentationas merely this dispatcher on top of them. Every function I've extracted has been pulled out with as few modifications as I can possibly arrange. The goal here is not to simplify the functions themselves, nor to correct regexen, nor to fix any bugs or strange behaviours. It is only to make it possible to thoroughly test the indentation entrypoint, and thus to document what we believe the current behaviour of the system is such that future PRs can modify that behaviour on purpose.I tried to cover reasonable test cases for each computation function; if any reviewers have code snippets/specific test cases they think we'd benefit from, I'd be delighted to incorporate them.
I believe this PR closes #41.