Skip to content
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

Add infrastructure for formatting specific line ranges #1007

Merged
merged 11 commits into from
May 31, 2016

Conversation

kamalmarhubi
Copy link
Contributor

This series of commits adds infrastructure for restricting formatting to specific line ranges, controlled by the --file-lines option. It includes a rough demonstration of formatting for statements. Later commits will extend this to more AST node type and refine the formatting more.

The goal is to make incremental formatting possible, and allow improving the formatting of a codebase one change at a time. This is much better than requiring that an entire project be formatted in one go in order to get the benefits of rustfmt. In later commits, we can add a tool to format a project based on a diff which will allow adding rustfmt to, eg, a git pre-commit hook without risk of introducing unrelated changes across a codebase.

There is some heuristic work needed to follow on from this to enable incremental formatting of all AST node types. For some, we will want to format when a change overlaps the AST node; for others, only when the change fully contains the AST node. As an example, we won't want to reformat an entire function because one line in it changed, but we would want to reformat an entire function signature if part of it changed. This will be done in later PRs.

Outline of the commits in the series:

  • commits 1-2 improve debuggability
  • commit 3 moves some codemap related utilities to their own module
  • commit 4 adds utilities for looking up line ranges in the codemap
  • commit 5 adds the FileLines struct, which represents a set of lines in files
  • commit 6 adds a proof of concept for formatting statements within the specified line ranges
  • commit 7 adds a command line option for specifying line ranges to be formatted

Refs #434

@kamalmarhubi kamalmarhubi force-pushed the basic-line-ranges-v2 branch from f134731 to 8b40f66 Compare May 26, 2016 13:40
@kamalmarhubi
Copy link
Contributor Author

kamalmarhubi commented May 26, 2016

This replaces #959. As a happy bonus, it's about 200 lines lighter.

@@ -0,0 +1,39 @@
use syntax::codemap::{BytePos, CodeMap, Span};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing copyright notice.

Also could you add a comment for what belongs in this file please? I think there is potential for confusion with FileMap and so forth.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@nrc
Copy link
Member

nrc commented May 30, 2016

lgtm, a few minor things/questions, but r+ with them addressed

@kamalmarhubi
Copy link
Contributor Author

@nrc thanks! Addressed some, responded to others.

Also, I'd appreciate one round of optional bike shedding on the --flie-lines flag name. I'm not good at names, and just picked something somewhat descriptive, but not entirely clear. :-)

@nrc
Copy link
Member

nrc commented May 30, 2016

I think --file-lines is fine, at least, I can't think of anything better.

Thanks for the changes. Needs a rebase before I can merge it.

This commit adds a `codemap` module, and moves the `CodemapSpanUtils`
added in rust-lang#857 to it. This is preparation for adding more `Codemap`
specific utilities.

Refs rust-lang#434
This commit adds extension methods to `Codemap` to allow looking up line
ranges for spans.

Refs rust-lang#434
This commit adds a type to represent lines in files, and adds it to the
`Config` struct. It will be used for restricting formatting to specific
lines.

Refs rust-lang#434
@kamalmarhubi kamalmarhubi force-pushed the basic-line-ranges-v2 branch from 4d94ab1 to b89670d Compare May 30, 2016 23:26
This commit adds a very rough implementation of handling the specified
line ranges in `config.file_lines_map` for statements. It reformats a
statement if its span is fully contained in the set of lines specified
for the file.

The implementation here is intended as a proof of concept, and
demonstration that the machinery added in the preceding commits is
functional. A final implementation would likely hook in via the
`Rewrite` trait.

Refs rust-lang#434
This commit adds the `--experimental-file-lines` option to rustfmt. This
allows specifying line ranges to format from the command line.

Refs rust-lang#434
This adds a note to both the `--config-help` output for `file_lines`,
and to the panic message on attempting to deserialize a `FileLines`
struct.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@kamalmarhubi kamalmarhubi force-pushed the basic-line-ranges-v2 branch from b89670d to e252100 Compare May 30, 2016 23:36
@kamalmarhubi
Copy link
Contributor Author

Rebased. A couple of items are left for future PRs:

  • decision on closed or half-open ranges
  • handling <stdin>

@nrc nrc merged commit 240dba5 into rust-lang:master May 31, 2016
@nrc
Copy link
Member

nrc commented May 31, 2016

Merged, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants