Upstream: #688

Description

Fixes issue #680. Handles the case where a list item is created with an empty string, which is not split by the lines iterator.

Testing guidelines

New test can be ran with cargo test

Checklist

• [x] I have read the contributing guidelines.
• [x] I have added relevant tests.
• [x] I have documented all new additions.

First of all, thank you for making this project revive. I'm suggesting to enable the discussions on this repository. Indeed, people may want to ask questions, share news of this project, and perhaps announce that the original owner came back. Well, it will be a great opportunity to exchange about this project.

Upstream: #689

Description

Testing guidelines

I checked all examples by cargo run --example {example-name} and confirmed they work on iTerm2 on my Mac machine.

Checklist

• [x] I have read the contributing guidelines.
• [x] I have added relevant tests. (Since this PR just upgrading existing dependency, no tests need to be added)
• [x] I have documented all new additions.

Description

crossterm v0.26.0 was released recently. And some people are starting to use the latest version. Since this crate requires crossterm 0.25, both 0.25 and 0.26 are mixed in user's project when the user depends on crossterm directly. I actually saw an issue around this in my tui-textarea project: https://github.com/rhysd/tui-textarea/issues/9

The best way to solve the issue is to follow the latest crossterm version by tui crate. I checked the release note and tui crate. Breaking changes seem not relevant to tui crate.

This is a breaking change due to MSRV bump from 1.56 to 1.58. crossterm 0.26 is using inline format arguments feature and it requires 1.58.

Upstream: #585

Main changes

I found that the current implementation of Layout was very heavy on allocations and clones, especially considering you're expected to use it plus the split() function every frame. So, I made some changes to allow Layout to work more on the stack.

This is what I changed Layout to

#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct Layout<'a> {
    direction: Direction,
    margin: Margin,
    constraints: &'a [Constraint],
    /// Whether the last chunk of the computed layout should be expanded to fill the available
    /// space.
    expand_to_fill: bool,
}

This replaces the constraints field with a reference to a slice, instead of a Vec<Constraint>. Considering that most implementations using Layout use static arrays anyway, this seemed like a no brainer, until I tried to use the LAYOUT_CAHCE. It is impossible to store this new Layout inside of the LAYOUT_CACHE because that would require that you borrow the given Layout for 'static. My solution to the problem was only storing the hash of the Layout into the cache as opposed to moving the entire Layout.

macro_rules! hash_layout {
    ($self:expr, $area:expr) => {{
        let mut to_hash = ahash::AHasher::default();
        $area.hash(&mut to_hash);
        $self.margin.hash(&mut to_hash);
        $self.expand_to_fill.hash(&mut to_hash);
        $self.direction.hash(&mut to_hash);
        $self.constraints.iter().copied().for_each(|f| match f {
            Constraint::Max(max) => to_hash.write_u16(max),
            Constraint::Min(min) => to_hash.write_u16(min),
            Constraint::Ratio(left, right) => {
                to_hash.write_u32(left);
                to_hash.write_u32(right);
            }
            Constraint::Length(length) => to_hash.write_u16(length),
            Constraint::Percentage(percentage) => to_hash.write_u16(percentage),
        });
        to_hash.finish()
    }};
}

#[derive(Clone, Copy)]
#[repr(transparent)]
struct CustomHash(u64);

impl Default for CustomHash {
    #[inline]
    fn default() -> Self {
        Self(0)
    }
}

impl std::hash::Hasher for CustomHash {
    #[inline]
    fn finish(&self) -> u64 {
        self.0
    }

    #[inline]
    fn write(&mut self, _: &[u8]) {
        panic!("unsupported operation");
    }

    #[inline]
    fn write_u64(&mut self, i: u64) {
        self.0 = i;
    }
}

thread_local! {
    static LAYOUT_CACHE: RefCell<HashMap<u64, Vec<Rect>, BuildHasherDefault<CustomHash>>> = RefCell::new(HashMap::default());
}

pub fn split(&self, area: Rect) -> Vec<Rect> {
    // TODO: Maybe use a fixed size cache ?
    LAYOUT_CACHE.with(|c| {
        c.borrow_mut()
            .entry(hash_layout!(self, area))
            .or_insert_with(|| split(area, self))
            .clone()
    })
}

This is the code I used for storing the layout. Here is a breakdown

• First I create a hasher using ahash, It's not critical that ahash be the hashing algorithm (the default hasher works as well). However, considering how often the split() function will be run, I think it's worth the sacrifice of one extra dependency.
• After creating the hasher, I feed it the area parameter, and then each field of the Layout one by one. cargo +nightly bench insisted that this was faster than simply running self.hash(&mut to_hash);
• After computing the hash of area + self, I use the final value as the final key into the LAYOUT_CAHCE.
• Because LAYOUT_CACHE uses CustomHash as it's RandomState, the hash computed by ahash is simply passed through and no further hashing is done. There's no point in hashing a hash.
• Finally, the resulting Vec<Rect> is either grabbed from the LAYOUT_CACHE or computed, and then cloned to bypass the reference created by the HashMap

Benchmarks

I did some benchmarks to see just how much my changes effected performance. This was my benchmark

#[bench]
fn bench_vertical_split_by_height(b: &mut test::Bencher) {
    let target = Rect {
        x: 2,
        y: 2,
        width: 10,
        height: 10,
    };

    let layout = Layout::default()
        .direction(Direction::Vertical)
        .constraints(
            [
                Constraint::Percentage(10),
                Constraint::Max(5),
                Constraint::Min(1),
            ]
            .as_ref(),
        );

    b.iter(|| layout.split(target));
}

When I benchmarked the original implementation and mine, these were my results. (top one is my implementation).

test layout::tests::bench_vertical_split_by_height ... bench:          89 ns/iter (+/- 5)
test layout::tests::bench_vertical_split_by_height ... bench:         243 ns/iter (+/- 2)

My implementation seems to be ~3x faster than the original implementation.

Swapping out ahash with the default hasher (std::collections::hash_map::DefaultHasher::default()) gave me this result

test layout::tests::bench_vertical_split_by_height ... bench:         148 ns/iter (+/- 10)

Smaller changes

I noticed that most of the methods inside of this module (layout) were very simple and small, so I made them const and #[inline]ed them.

Pre-requisites

All of my changes passed the unit tests and ran the examples.

Thanks for considering my PR, - StratusFearMe21

Hello all, I've been following https://github.com/fdehau/tui-rs/issues/654 with interest!

I maintain an RSS/Atom reader called Russ that is built on tui-rs (please feel free to add it to the list of projects using tui-rs, as I am extremely pleased with and grateful for tui-rs).

My question is this: given the fork, with regard to bug reports/feature requests/PRs, should I file those here, or upstream? Is the plan to merge the work in this fork upstream, or have this fork eventually replace upstream? Thank you to everyone actively working to keep tui-rs going, it's such a lovely project.

Description

Currently CI reports 3 kinds of deprecation warnings:

• Node.js 12 actions are deprecated. Please update the following actions to use Node.js 16: hecrj/setup-rust-action@967aec96c6a27a0ce15c1dac3aaba332d60565e2, actions/cache@v2.
• The set-output command is deprecated and will be disabled soon. Please upgrade to using Environment Files.
• The save-state command is deprecated and will be disabled soon. Please upgrade to using Environment Files.

This PR fixes all the warnings.

Testing guidelines

I checked all warnings are no longer reported on my forked repository: https://github.com/rhysd/tui-rs-revival/actions/runs/4191656530

Checklist

• [x] I have read the contributing guidelines.
• [x] I have added relevant tests.
• [x] I have documented all new additions.

Description

Add poketex to 'apps using tui' in README.

Testing guidelines

Checklist

• [x] I have read the contributing guidelines.
• [x] I have added relevant tests.
• [x] I have documented all new additions.

Description

Links to some 3rd party libraries including my tui-textarea crate were accidentally removed at #21. This PR fixes them.

Testing guidelines

Nothing to test since this is a small document change.

Checklist

• [x] I have read the contributing guidelines.
• [x] I have added relevant tests.
• [x] I have documented all new additions.

Description

This PR adds macOS to CI as one of major platforms which should be supported by tui crate. The CI workflow was also parametrized to reduce duplicate steps for each OS.

Testing guidelines

I confirmed all jobs passed in my forked repository: https://github.com/rhysd/tui-rs-revival/actions/runs/4200844754

Checklist

• [x] I have read the contributing guidelines.
• [x] I have added relevant tests.
• [x] I have documented all new additions.

Description

Fixes #53. This PR re-enables clippy on CI, which was disabled at #51.

Testing guidelines

I confirmed all checks passed in my forked repository: https://github.com/rhysd/tui-rs-revival/actions/runs/4193808784/jobs/7271163234

Checklist

• [x] I have read the contributing guidelines.
• [x] I have added relevant tests.
• [x] I have documented all new additions.

Description

#51 bumped MSRV, but rust-version field of Cargo.toml is still 1.56.1.

Testing guidelines

Since this only changes the field in Cargo.toml, I checked only cargo clippy

Checklist

• [x] I have read the contributing guidelines.
• [x] I have added relevant tests.
• [x] I have documented all new additions.

I'm not a user of helix or tui. But I found this helix issue and thought I'd mention it here: https://github.com/helix-editor/helix/issues/6012

Helix uses a fork of a subset of tui. Notably the core of Buffer is basically the same (AFAIK).

I'm pretty sure that this bug is shared by everything using the tui-style Buffer. AFAIK, the reason is that unicode-width doesn't (or can't?) report the column width for emojis. See readme and issues: https://github.com/unicode-rs/unicode-width

I won't have anything to contribute here, or any personal interest, really. Perhaps you should get into contact with the helix team for this, as this effects both.

Description

https://github.com/tui-rs-revival/tui-rs-revival/pull/22#issuecomment-1427143924 introduced the idea to return &'static [Rect] on Layout::split using unsafe, but we can use a safe abstraction here instead with Rc.

Testing guidelines

Using the benchmark introduced in #22, I get these results (master/this respectively)

test layout::tests::bench_vertical_split_by_height ... bench: 129 ns/iter (+/- 6)
test layout::tests::bench_vertical_split_by_height ... bench: 100 ns/iter (+/- 3)

Checklist

• [X] I have read the contributing guidelines.
• [X] I have added relevant tests.
• [ ] I have documented all new additions.

Problem

The word master has been deprecated in favor of main branch, but this repo defaults to master branch.

Solution

Switch to main branch.

Note: the prs should auto update to main branch, but might take some time to update.

Problem

Right now there isn't a specific convention for commits and adopting conventional commits would be nice.

Solution

Utilize a tool for checking the PRs for conventional commits. Such as action-semantic-pull-request or cocogitto.

Alternatives

None.

Additional context

• https://marcodenisi.dev/en/blog/why-you-should-use-conventional-commits/

Upstream: #677

Description

It is a PR just to resolve issue #352

Add a few lines to explain how to define the coordinates

Testing guidelines

Just test it using VS Code

Checklist

• [X] I have read the contributing guidelines.
• [X] I have added relevant tests.
• [X] I have documented all new additions.