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

[DNM] #465

Closed
wants to merge 11 commits into from
Closed

[DNM] #465

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 4 additions & 11 deletions .github/workflows/preview-release.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ on:
- completed
push:
# Run only on trunk pushes that aren't a new tag release
branches: [trunk]
branches: [trunk, reese-hack-special-release]
tags-ignore: "*"

env:
Expand All @@ -30,7 +30,7 @@ jobs:
level: prepatch
- uses: actions-ecosystem/action-push-tag@v1
with:
tag: ${{ steps.bump-semver.outputs.new_version }}
tag: v0.10.19-0
build:
runs-on: ${{ matrix.os }}
needs: [bump-tag]
Expand Down Expand Up @@ -64,16 +64,13 @@ jobs:
override: true
profile: minimal
target: aarch64-unknown-linux-gnu
- uses: actions/cache@v2
with:
path: |
librubyfmt/ruby_checkout
key: ${{ runner.os }}-${{matrix.target}}-ruby-v1-${{ hashFiles('.git/modules/librubyfmt/ruby_checkout/HEAD') }}
- if: runner.os == 'macOS'
run: |
brew install automake bison
echo "/usr/local/opt/bison/bin:$PATH" >> $GITHUB_PATH
- run: ./script/make_release
env:
TARGET: ${{ matrix.target }}
- uses: actions/upload-artifact@v3
with:
name: rubyfmt-release-artifact-${{ matrix.os }}-${{ matrix.target }}
Expand Down Expand Up @@ -109,10 +106,6 @@ jobs:
- uses: actions/download-artifact@v3
with:
name: rubyfmt-release-artifact-ubuntu-20.04-aarch64-unknown-linux-gnu
- run: |
# The arch part of this path is set with uname, but we cross-compile the arm build on
# an x86 machine, so we want to make sure the name is correct for the release
mv rubyfmt-${{ steps.get-latest-tag.outputs.tag }}-Linux-x86_64.tar.gz rubyfmt-${{ steps.get-latest-tag.outputs.tag }}-Linux-aarch64.tar.gz
- uses: actions/download-artifact@v3
with:
name: rubyfmt-release-artifact-ubuntu-20.04-native
Expand Down
10 changes: 10 additions & 0 deletions fixtures/small/block_param_line_length_actual.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
ThisIsAReallyLongClassName::WithSomeModulesInsideItThatHaveAMethodThatWeWillCallRightAroundHereeeee.do_stuff(boom) do |foo|
some_stuff!
end

ThisIsAReallyLongClassName::ButSlightShorterWithMoreCalls.foo.bar.baz.quux.what_comes_after_quux_idk_aaaahhhh.map { |model|
body_of_the_call
}

ThisIsAReallyLongClassName::ButSlightShorterWithMoreCalls.foo.bar.baz.quux.what_comes_after_quux_idk_aaaahhhhhhh.map(&:foo)
ThisIsAReallyLongClassName::ButSlightShorterWithMoreCalls::ThisIsAReallyLongClassName::ButSlightShorter.new(foo: bar_baz_quuz)
25 changes: 25 additions & 0 deletions fixtures/small/block_param_line_length_expected.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
ThisIsAReallyLongClassName::WithSomeModulesInsideItThatHaveAMethodThatWeWillCallRightAroundHereeeee
.do_stuff(boom) do |foo|
some_stuff!
end

ThisIsAReallyLongClassName::ButSlightShorterWithMoreCalls
.foo
.bar
.baz
.quux
.what_comes_after_quux_idk_aaaahhhh
.map { |model|
body_of_the_call
}

ThisIsAReallyLongClassName::ButSlightShorterWithMoreCalls
.foo
.bar
.baz
.quux
.what_comes_after_quux_idk_aaaahhhhhhh
.map(&:foo)
ThisIsAReallyLongClassName::ButSlightShorterWithMoreCalls::ThisIsAReallyLongClassName::ButSlightShorter.new(
foo: bar_baz_quuz
)
25 changes: 13 additions & 12 deletions fixtures/small/long_blockvar_expected.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
things.each do |
omg:,
really:,
dang:,
long:,
blockvar:,
that_is_so_long_if_you_write_this:,
you_should_refactor:,
like_really_this_is_so_long:
|
do_things!
end
things
.each do |
omg:,
really:,
dang:,
long:,
blockvar:,
that_is_so_long_if_you_write_this:,
you_should_refactor:,
like_really_this_is_so_long:
|
do_things!
end
5 changes: 5 additions & 0 deletions fixtures/small/multiline_chain_in_block_actual.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,8 @@
def ajax_get(route)
super
end

class Foo
sig {override.returns(T::Array[T.class_of(Some::Really::Long::Name::ThatshouldprobablybealisedbutisntbecauseThis::IsATestStub)])}
def example = begin; end
end
10 changes: 10 additions & 0 deletions fixtures/small/multiline_chain_in_block_expected.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,13 @@
def ajax_get(route)
super
end

class Foo
sig {
override.returns(
T::Array[T.class_of(Some::Really::Long::Name::ThatshouldprobablybealisedbutisntbecauseThis::IsATestStub)]
)
}
def example = begin
end
end
4 changes: 2 additions & 2 deletions librubyfmt/src/delimiters.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::line_tokens::ConcreteLineToken;

#[derive(Debug, Clone)]
#[derive(Debug, Clone, Eq, PartialEq)]
struct DelimiterPair {
open: String,
close: String,
Expand All @@ -12,7 +12,7 @@ impl DelimiterPair {
}
}

#[derive(Debug, Clone)]
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct BreakableDelims {
single_line: DelimiterPair,
multi_line: DelimiterPair,
Expand Down
59 changes: 45 additions & 14 deletions librubyfmt/src/render_targets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,38 +252,69 @@ impl AbstractTokenTarget for BreakableCallChainEntry {
// individual line and get _that_ max length.
let mut tokens = self.tokens.clone();
if tokens.len() > 2 {
if let Some(AbstractLineToken::ConcreteLineToken(ConcreteLineToken::End)) =
tokens.get(tokens.len() - 2)
{
// Pop off all tokens that make up the `do`/`end` block (but not `do`!),
let index = tokens.len() - 2;
let token = tokens.get_mut(index).unwrap();
if matches!(
token,
AbstractLineToken::ConcreteLineToken(ConcreteLineToken::End)
) {
// Pop off all tokens that make up the block (but not the block params!),
// since we assume that the block contents will handle their own line
// length appropriately.
while let Some(token) = tokens.last() {
if matches!(
token,
AbstractLineToken::ConcreteLineToken(ConcreteLineToken::DoKeyword)
AbstractLineToken::BreakableEntry(BreakableEntry { delims, .. }) if *delims == BreakableDelims::for_block_params()
) {
break;
}
tokens.pop();
}
} else if let AbstractLineToken::BreakableEntry(BreakableEntry {
delims,
ref mut tokens,
..
}) = token
{
if *delims == BreakableDelims::for_brace_block() {
if let Some(AbstractLineToken::BreakableEntry(BreakableEntry {
delims, ..
})) = tokens.first()
{
if *delims == BreakableDelims::for_block_params() {
// Wipe away the body of the block and leave only the params
*tokens = vec![tokens.first().unwrap().clone()];
} else {
// No params, so wipe away the whole thing
*tokens = Vec::new();
}
} else {
// No params, so wipe away the whole thing
*tokens = Vec::new();
}
}
}
}

if let Some(AbstractLineToken::BreakableEntry(_)) = tokens.first() {
tokens.remove(0);
}
// EndCallChainIndent, which we don't care about
tokens.pop();
// If the last breakable extends beyond the line length but the call chain doesn't,
// the breakable will break itself, e.g.
// ```ruby
// # ↓ if the break is here, we'll break the parens instead of the call chain
// AssumeThisIs.one_hundred_twenty_characters(breaks_here)
// ```
if let Some(AbstractLineToken::BreakableEntry(_)) = tokens.last() {
if let Some(AbstractLineToken::ConcreteLineToken(ConcreteLineToken::EndCallChainIndent)) =
tokens.last()
{
tokens.pop();
}
let call_count = tokens.iter().filter(|t| matches!(t, AbstractLineToken::ConcreteLineToken(ConcreteLineToken::Dot | ConcreteLineToken::LonelyOperator))).count();
// If the last breakable is multiline (and not a block/block params), ignore it. The user likely
// intentionally chose a line break strategy, so try our best to respect it.
//
// However, if there's only one item in the chain, try our best to leave that in place.
// `foo\n.bar` is always a little awkward.
if let Some(AbstractLineToken::BreakableEntry(be)) = tokens.last() {
if (call_count == 1 || be.is_multiline()) && be.delims != BreakableDelims::for_brace_block() && be.delims != BreakableDelims::for_block_params() {
tokens.pop();
}
}
tokens.insert(
0,
AbstractLineToken::ConcreteLineToken(
Expand Down
2 changes: 1 addition & 1 deletion script/make_release
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ case "$target" in
TARGET_AR=aarch64-linux-gnu-ar \
cargo build --release --target aarch64-unknown-linux-gnu

cargo_target_dir_prefix="aarch64-unknown-linux-gnu"
cargo_target_dir_prefix="aarch64-unknown-linux-gnu/"
# This is kind of a hack, since we're assuming we're on a Linux host.
release_tarball_os="Linux"
release_tarball_arch="aarch64"
Expand Down
Loading