Pad filenames with zero for day solutions

[logan-connolly]

Again, really awesome work here! This project is great.

problem

The issue I discovered was that the day files aren't padded when day < 10. This causes sorting issues when listing files locally or viewing them in the code forge. This is really a minor issue, but I think it would be a nice improvement.

# current output
day_1.gleam
day_10.gleam
day_11.gleam
day_2.gleam

proposal

Users would be able to activate a pad (not sure on naming) setting in the gleam.toml which would create files with padding when running gladvent run new ... and expect padding when loading files with gladvent run run ....

[gladvent]
pad = true

The important thing is to not break current setups, which is why when this setting is false or not available, the default behavior would be to not use padding. Here is what listing the files looks like when sorting with padding = true:

# proposed output
day_01.gleam
day_02.gleam
day_10.gleam
day_11.gleam

deprecation (optional)

If you think, this is a good default behavior and want to implement it in v3, you could consider adding a deprecation warning when users run gladvent run new ... if the pad setting is not set in gleam.toml. That way users are made aware of the feature and can create new solutions with padding.

In addition, a gladvent run migrate command could be added. In v2, this would basically iterate over all solutions, check if padding exists, if not pad file name.

Then in the v3, the conditional logic for pad would be dropped, and either drop or adapt the migrate command, i.e. the migrate command would remove the pad setting in the config.toml if it exists (don't know how trivial this would be).

wip

I got hooked last night digging into the code and started to implement this locally. It seems to be totally feasible.

A nice feature of int.parse in Gleam is that "01" is parsable which makes the runner logic largely untouched:

import gleam/int
import gleeunit/should

pub fn convert_padded_day_to_int_test() {
  "01" |> int.parse() |> should.be_ok() |> should.be_equal(1)
}

And simply adding a helper for getting the day as a string could look like:

pub fn day_to_string(day: Day, pad pad: Bool) -> String {
  case pad {
    True -> int.to_string(day) |> string.pad_start(to: 2, with: "0")
    False -> int.to_string(day)
  }
}

roadmap

If you are happy with the proposal, I'd like to further work on this feature. I see it being done in the following steps (happy to get feedback from you):

1. add pad parameter to creating source files and reading from source paths (explicitly set to false)
2. add ability to read pad option from gleam.toml, injecting the flag it into the read/write helpers
3. (optional): add deprecation warnings when pad setting not set and the user creates a new solution
4. (optional): add migrate command that migrates existing solutions to padded format
5. (optional): post v3 migration, clean up conditional logic in previous stages and drop/adapt migrate.

These would all be done as individual pull requests. Looking forward to getting some feedback :)

[TanklesXL]

Oh hey that's not a bad idea, but I would simplify it a bit and say that we do all of this in v3 and omit the configuration aspect.

My suggestion is that padding be required to run v3 onward, and the command to update (whether it is called migrate/upgrate/wtv) be added in v3 and removed in an eventual v4.

That way as of v3 we always generate new days with the correct padding, but can prompt the user to run the upgrade command if we find they're trying to run days that don't have their names padded. This should be relatively simple because we can check if they expose day_x and not day_0x fairly easily

In addition, I would apply the padding to the names of input text files as well as the gleam files for consistency.

What do you think?

[TanklesXL]

Actually now that I think about it, with making the check at run rather than at new we can bundle this all into v2 as you originally suggested. And then remove the command in v3

In v2 we can run either the padded or unpadded days and provide the prompt to run the migrate command if we notice the old format is being used.

[logan-connolly]

Hey, thanks a lot for the feedback!

I quite like the approach from your initial comment a lot. It would make things so much simpler. Since we would already be checking for outdated file patterns, we could process normally and output a deprecation message:

Legacy file detected. In v4 this will no longer be supported. Please run gleam run upgrade.

Or panic and tell them to run upgrade would work to! It is a major version change after all.

Targeting v3 seems like a cleaner cut, and users can always remain on v2 if they want. I don't like that my initial approach called for changing the output of gleam run new ... in v2. Although not a breaking change with the config, it is quite a significant change in behavior.

In addition, I would apply the padding to the names of input text files as well as the gleam files for consistency.

Good point! Noted.

If you are good with it, I'd be in favor of targeting these changes to a v3 branch. Would also be much easier to develop :) what do you think?

[TanklesXL]

Yeah that works for me, I say go for it! I can make a branch for you to target

we could process normally and output a deprecation message

This is the way to go i think, we can be smart about what we run and there's no need to panic, though if it turns out to be too complicated inside the normal runner flow then we can have a normal error returned and printed

[TanklesXL]

@logan-connolly you can point your changes at the next branch: https://github.com/TanklesXL/gladvent/tree/next