Skip to content

Conversation

@Noologos
Copy link
Contributor

@Noologos Noologos commented Jun 7, 2025

This PR introduces improved folder sorting capabilities:
Build List:
Folders are now sorted by modification time (newest first) when "Sort by Last Edited" is active.
For other sort modes (Name, Class, Level), folders are sorted by name (A-Z).
Files continue to sort based on the selected mode.
Folders are always listed before files.
Save As Dialog:
The folder list within the "Save As" dialog now inherits its sorting behavior from the main build list's current Sort by setting (main.buildSortMode).
If "Sort by Last Edited" is active in the main list, folders in the "Save As" dialog are sorted by modification time (newest first).
Otherwise, folders in the "Save As" dialog are sorted by name (A-Z).

@LocalIdentity LocalIdentity added user-interface Changes that only affect the UI pob2 Label for features that should be ported over to PoB-PoE2 labels Jun 7, 2025
Copy link
Contributor

@Nightblade Nightblade left a comment

Choose a reason for hiding this comment

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

Please only use TAB indentation to be consistent with the rest of the code.

Copy link
Contributor Author

@Noologos Noologos left a comment

Choose a reason for hiding this comment

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

Will do

@Noologos
Copy link
Contributor Author

Noologos commented Jun 9, 2025

I suspect that it would be trivial to add a format checker to the workflows to speed up these kinds of issues in the future.

@Nightblade
Copy link
Contributor

You're welcome to submit a format checker workflow! 😃

@Noologos
Copy link
Contributor Author

Noologos commented Jun 9, 2025

You're welcome to submit a format checker workflow! 😃

for sure, do you folks have format guidelines somewhere? I did not see it in the Contributing.md.

@Nightblade
Copy link
Contributor

There are no official format guidelines; we generally just try to be consistent with the existing code. Indentation is probably the only area of concern as it's easy to overlook, especially for new contributors.

@Paliak
Copy link
Contributor

Paliak commented Jun 9, 2025

I have an ancient branch that attempted to implement luacheck/LuaFormatter among a bunch of other changes if you need inspiration:

https://github.com/Paliak/PathOfBuilding/blob/a34de95910cec3db4ce22600935820d96b817289/Dockerfile
https://github.com/Paliak/PathOfBuilding/blob/a34de95910cec3db4ce22600935820d96b817289/.luacheckrc
https://github.com/Paliak/PathOfBuilding/blob/a34de95910cec3db4ce22600935820d96b817289/.lua-format

a variation of the docker file is a part of the repo now but it doesn't include luacheck/LuaFormatter.

@Noologos
Copy link
Contributor Author

Noologos commented Jun 9, 2025

I have an ancient branch that attempted to implement luacheck/LuaFormatter among a bunch of other changes if you need inspiration:

https://github.com/Paliak/PathOfBuilding/blob/a34de95910cec3db4ce22600935820d96b817289/Dockerfile https://github.com/Paliak/PathOfBuilding/blob/a34de95910cec3db4ce22600935820d96b817289/.luacheckrc https://github.com/Paliak/PathOfBuilding/blob/a34de95910cec3db4ce22600935820d96b817289/.lua-format

a variation of the docker file is is a part of the repo now but it doesn't include luacheck/LuaFormatter.

how do we feel about https://github.com/marketplace/actions/stylua it should be fairly simple to add a github action to run a pass on all prs that modify or add lua files.

@Noologos Noologos requested a review from Nightblade June 9, 2025 11:02
@Paliak
Copy link
Contributor

Paliak commented Jun 9, 2025

It's a 3rd party action which kind of rubs me the wrong way security wise. Also i really like that we're not completely reliant on github actions for anything and things can work be ran locally without too much hassle (having to use stuff like act etc).

At the end of the day it's up to the maintainers tho.

@Nightblade
Copy link
Contributor

I had a very quick look at Stylua. To me it didn't seem like a good fit -- It looked more like a formatter than a linter, and its output seemed limited to (huge) diffs.

Copy link
Member

@Wires77 Wires77 left a comment

Choose a reason for hiding this comment

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

Looks good to me, I just had one question on it.

RE: Linter/Formatter, I was attempting to use the same tools Paliak referred to to keep the codebase consistent with itself while not introducing a crazy huge diff at the beginning. I also opened a draft PR forever ago with some basic guidelines, but the config Paliak referenced covers some of that too.

I don't have a bias toward or against any tool, really, as long as it accomplishes the above goals.

@Wires77 Wires77 merged commit dfb6ce3 into PathOfBuildingCommunity:dev Jun 13, 2025
github-actions bot pushed a commit to PathOfBuildingCommunity/PathOfBuilding-PoE2 that referenced this pull request Jun 13, 2025
Wires77 pushed a commit to PathOfBuildingCommunity/PathOfBuilding-PoE2 that referenced this pull request Jun 13, 2025
Co-authored-by: Noologos <Noologos@users.noreply.github.com>
Tonkat pushed a commit to Tonkat/PathOfBuilding-PoE2 that referenced this pull request Sep 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pob2 Label for features that should be ported over to PoB-PoE2 user-interface Changes that only affect the UI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants