-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Further cleanup #223
Merged
Merged
Further cleanup #223
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.
I've built a whole bunch of new testing aaaaaand it has uncovered things that don't work! This PR is already big enough, so I'm adding these tests commented out, and will start bugfixes in the next PR.
Thanks for many thanks for all the work and sorry for the delay (lot of microcommits) |
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.
Note: This PR is the fourth in a series starting with #219. It continues work detailed in #218.
What's here?
This PR is the last of my "notes and documentation" PRs. It adds more tests and more code-comments to help orient further work. However: I've also hit the point where testing is beginning to turn up bugs. To keep bugfix and improvement PRs short and to the point, this PR includes some disabled tests. The next PR will be the first PR that starts deeply changing the meaning of the code, rather than simply organization and naming.