Scale IME position by hidpi_factor.

[awused]

Fixes #2056

[chrisduerr]

Thanks a ton for fixing this, the functionality looks good to me so far.

This still needs a change log entry before it can be merged.

I'd also like to resolve some style issues with this method since this is already related to it. I think it would be nice if you could move the imports in that method to the beginning of the file.

I think the full method body should look somewhat like this:

        let point = terminal.cursor().point;
        let SizeInfo {
            cell_width: cw,
            cell_height: ch,
            padding_x: px,
            padding_y: py,
            ..
        } = *terminal.size_info();
        let dpr = self.window().hidpi_factor() as f32;

        let nspot_y = (py + (point.line.0 + 1) as f32 * ch / dpr) as i32;
        let nspot_x = (px + point.col.0 as f32 * cw / dpr) as i32;
        self.window()
            .set_ime_spot(LogicalPosition::from((nspot_x, nspot_y)));

I think this would improve readability especially for people unfamiliar with struct destructuring.

[awused]

Made requested changes.

[chrisduerr]

The .. should be on the next line.

[awused]

Is there a style guide? You don't seem to run rustfmt, and I'm not familiar with rust conventions.

[nixpulvis]

I haven't seen a style guide for this repo, but generally all Rust examples I've seen, and more generally across languages it's rare to put all items separated by a comma on separate lines except the last one, even if it's an .. like thing.

The way it is now, it's too easy to miss.

[nixpulvis]

FWIW, there are many Rust style guide in the community, the most official (and incomplete) probably being https://doc.rust-lang.org/1.0.0/style/, however it doesn't mention this case.

[chrisduerr]

Generally I try to follow rustfmt and I plan to run it on the complete repo some time in the future. However since there are constantly new PRs coming in, this is extremely difficult to do.

[awused]

As long as you're asking people to do things manually there are going to be differences and omissions. In this case I would normally have put .. on its own line but since I had to format it manually I missed it, which is why I asked if there were a style guide so I could at least have a set of rules to keep in mind.

@nixpulvis Do you mean now as in how the code is now, or now as in how it was when this comment thread started?

[chrisduerr]

As long as you're asking people to do things manually there are going to be differences and omissions

Which is why I've pointed it out in the review. It does put an additional strain on the review process and it's easier to miss things, however just running rustfmt on everything isn't as simple as it might sound at first.

Though the plan to run rustfmt came up originall when there were like 40/50+ open PRs. So we're getting closer and closer to an acceptable number of pending changes to put through something like this.

Approaching it file-by-file might be another option worth considering. Though all of this involves a lot of effort, so right now just manually reviewing the formatting is the path of least resistance.

Also please note that all of this is indeed documented here.

[nixpulvis]

@awused the way it was.

[awused]

I only skimmed the documentation since you asked me to make this three line change myself, so I did miss that. But since you're planning to run rustfmt anyway I'm not sure it was worth the time to restyle existing code in such a small PR.

[chrisduerr]

Thanks for the PR. Everything's looking good to me now.

Waiting for CI to finish, then it should be ready to be merged.