Improve indentation robustness #48
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
|
Wanted to say thanks to you. You fixed very important thing to many Nix* people. Yes, "track function definitions/calls" in Nix is obscure and can be hard even for a human. |
nix-mode.el
Outdated
| "Return regexp for matching string quotes." | ||
| (nix-mode-make-regexp nix-mode-quotes)) | ||
|
|
||
| (defun nix-mode-combined-regexp |
There was a problem hiding this comment.
This should have a parens.
| (when (/= counter 0) (goto-char (point-min))) t)) | ||
| (defun nix-mode-make-regexp (parts) | ||
| "Combine the regexps into a single or-delimited regexp." | ||
| (declare (indent defun)) |
There was a problem hiding this comment.
nix-mode-make-regexp takes a list of regexps and makes a larger regex out of them separated by \\| so that any of the constituents match.
|
Thanks so much for doing this! I think this is definitely an improvement and we are getting closer to having things working properly. A few notes trying this on some nix expressions:
|
That is just how it is going to be unless someone wants to hack in some heuristics. Consider what the header is. An open I'll take a look at the "with" handling as that should be handled properly ( |
|
OK the
The |
|
I've started using this and have found the following strange behavior: { nodes, pkgs, ... }:
{
services.prometheus = {
scrapeConfigs = [
{
job_name = "node";
static_configs = [
{ targets = builtins.map
(nodename: "${nodename}:9100")
(builtins.attrNames nodes);
}
];
}
];
};
} |
|
Can we not leverage something like rnix to help improve indentation? |
|
As for trying this... |
|
@mkaito rnix looks interesting, check out It's been on my mind for a while now to use it as an aggressive Nix pretty-printer (like |
yorickvP
suggested changes
Sep 4, 2018
|
@mkaito said:
Sorry that should be fixed now. @grahamc said:
Sorry, I can't replicate. Copying your paste into a buffer and formatting I get: { nodes, pkgs, ... }:
{
services.prometheus = {
scrapeConfigs = [
{
job_name = "node";
static_configs = [
{ targets = builtins.map
(nodename: "${nodename}:9100")
(builtins.attrNames nodes);
}
];
}
];
};
} |
mkaito
suggested changes
Sep 4, 2018
nix-mode.el
Outdated
| (cond | ||
| ((looking-at (nix-mode-quotes-regexp)) | ||
| ;; skip over strings entirely | ||
| (re-search-backward nix-mode-quotes-regexp nil t)) |
There was a problem hiding this comment.
(re-search-backward (nix-mode-quotes-regexp) nil t))
matthewbauer
approved these changes
Sep 8, 2018
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.
Improves indentation by covering more cases more often. It isn't perfect, but I've shopped it around and people seem to approve.
I defer to the code, which is heavily commented, for a deeper explanation of the technique. But in summary, the algorithm looks something like:
capsandendswhich we will count to track expression balance.** Quirks
=sign in assignments must be couched by spaces