From what I can see, I'm fine with not squashing the commits.

As for alignment, ideally they should be aligned on the decimal point, and if they always print with two decimal values, this new alignment achieves the same, so it's better as far as I can tell

While you're here/"there" [in the code], I'd like to open for yet another feature creep: I'm not a fan of player not being able to sell beyond demand. Could it be possible to have the "demand" be just at that price, once it's reached 0, price is adjusted accordingly (and perhaps demand is > 0 again at this new price)? Or just reduce price by half or 20 % or similar if demand has reached constant 0?

Granted I haven't play tested the current market on master, but seems harsh for a trader to arrive and not be able to sell their inventory, but rather to have to jettison it (or fly to another station?).

I think adjusting price after demand has been met is out-of-scope for this PR and liable to generate some discussion. So I'd prefer to leave that for another PR. I can create a new issue in order to track it. But is it realistic that a market would buy a commodity, even discounted, that they don't want? The risk is on the trader.

Aligning on decimal point would require a fair bit more work. In this case, the only decimal values are "Money", which always have 2 decimal places, so it's a moot point.

Looking at the new display, I'm now thinking instead of "Buy Price" and "Sell Price" to simply head the columns as "Buy" and "Sell". It should be implicitly obvious it's the price given the monetary values. Thoughts?

Ommiting the Price from the header sounds good. Less clutter. Maybe even "In stock" could be just "Stock" and also "Cargo" could be changed to either "Hold" on In hold".

This generally looks good. The screen needs to be fully rewritten to use the new ImGui Tables feature, as well as move away from the hardcoded magic-constants design, but for an interim change this works well enough.

A few thoughts:

• The commodity name field is a bit excessively wide. Is this to accommodate e.g. the German translation?
• Since the dividing line between the two fields is no longer in the center, it should be at either 62% (golden ratio) or at 2/3rds across the screen.
• For examples regarding how "modern" UI screens should be structured, look at saveloadwindow.lua or the equipment outfitter UI on the equip-v2 branch. Those are the most up-to-date complete rewrites of a UI screen, and demonstrate the programming patterns I would like to see adopted across the entire UI.

I can take a look at rewriting it, thanks for the examples of how it should be written, but no idea when I'll get around to a larger-scale rework. I thought I was picking some low-hanging fruit ;)

Changing the ratio to 62%, let alone 2/3, reduces the right-side size even further, which causes the button-bar to look very squashed. Given this will be rewritten "properly" "soon" anyway, can we leave this PR as-is for now?

I can take a look at rewriting it [...] I thought I was picking some low-hanging fruit ;)

For clarity, I was mentioning it in the abstract rather than suggesting you should do it yourself - feel free to do so if you're interested though!

Displaying buy/sell price separately is definitely a low-hanging fruit and I'm glad you've tackled it.

Given this will be rewritten "properly" "soon" anyway

You're speaking my language now! 😄

can we leave this PR as-is for now?

That's fine. I don't know when exactly we'll want to fully redesign the screen, but likely not before the next release.

~~TODO: the BBS (black) markets need a similar update.. this will be a bit more challenging as they open in a smaller popup window.

Ideally the code also should be refactored so all commodity market displays use the same code.. ie, make a "commodity market" card whose input is the list of commodities and their prices to display, which can then render itself within any parent window.~~

NVM - it appears the BBS markets already use the main market code (I was playing with a branch which did not have these changes when I noticed it) - but it does require some tweaking, at least on 4:3 displays.

Ok I thought I had introduced the issue on the BBS markets that resizes the window to full-height as soon as a commodity item is selected and the buy/sell part of the interface is populated, but the issue already exists in the current code.

I have tried to fix this but cannot find the reason why the window expands as soon as a commodity item is clicked on. This may need deferring to a future PR when the entire screen is re-written using the new UI paradigm.

Before clicking on Commodity - window takes up partial screen:

After clicking on Commodity - window is expanded to take up entire vertical space and "Hang Up" button is not visible (must be scrolled):

I think the commodity market resizing out of control in a BBS ad is a known issue - I recall trying to fix it once before, but that either never made it to a PR or I never got as far as an actual fix.

Go ahead and defer it to another PR - I'll review this code as-is soon.