Added file name option %Nd to include auto incremented number to file name on Configure... > Save > Default Save Filename. It includes an option to pad the increment number to a specified number of digits and reseat the increment value upon restart. I added an example to the Configure page for clarity. This feature uses two new configuration items.
BUG: 358641
BUG: 373759
FIXED-IN: 18.12.0
| Lint Skipped |
| Unit Tests Skipped |
Like D15106, this doesn't apply against the current state of git master. See D15106#319771 for suggestions.
Let's try this diff. I also tidied up what will happen for large sequential file numbers - they roll over at 1,000,000.
Thanks again for this patch! Sorry it's taken so long for you to get a review.
Since as far as I can tell the number isn't actually optional, the caption currently isn't accurate. Of course, maybe it should be optional, in which case the caption could read like this:
"<b>%d</b>: Sequential numbering; add a digit before the "d" to pad the number with that many zeroes"
If we improve the presentation up here, we may not need a new example section down below, which has resulted in the settings window being sort of awkwardly tall.
Is the "Next sequential number" textfield really necessary? I feel like that may be a bit too technical and esoteric a feature (open to explanations, of course!).
| src/Gui/SettingsDialog/SaveOptionsPage.cpp | ||
|---|---|---|
| 40 | Unrelated change | |
| 62 | Unrelated change | |
| 114 | Needs a <br /> here | |
I agree about the help text on the use of this placeholder. I simplified it based on your suggestion. I think it is better but perhaps not perfect and am open to further suggestions on it.
The Regex for %\d*d will capture 0 or more numbers between the literals % and d. In my testing, the padding works as intended with a number, there is no padding with no number, and putting something else in between the % and d results in that string (like %ad) in the file name rather than attempting the replacement. Are you seeing something different?
I included the 'Next Sequential Numbering' field after adding the 'Restart numbering with relaunch' field. I like the 'Restart number...' option since I often inadvertently close the application after a screen shot with <ESC> and though it would be nice to keep my numbering current across launches as I work on a particular series of screen shots. To avoid relaunching just to reset the counter, I considered a button to reset the counter to 1 but then thought maybe a text field showing the next number to use would a) inform the user about what it will do next and b) allow for corrections to the sequential numbering if necessary (like re-dosing a screen shot). I realize this may be more complicated than is necessary so if you'd prefer that I back some of this out or put it somewhere else in the settings dialog, I'm all for it.
Thank you for the feedback. No worries on the review timeline.
This is better, thanks. Now the optional number padding works too. Maybe I was holding it wrong?
I think the string is still a bit confusing, especially with the addition of the braces that would actually break parsing if a user tried to add them into the template. How about this instead?
<b>%d</b>: Sequential numbering <b>%Nd</b>: Sequential numbering, padded with up to N zeroes
I'm still just not sure about the spinbox for changing the number. I could get behind the checkbox for whether or not to restart the numbering with each launch, which makes sense to me. It feels like we need better smarts to automatically guess the correct next number when Restart numbering with relaunch is unchecked, and the spinbox is a workaround for not having that. Thoughts?
Oh, and can we change "Restart numbering with relaunch" to "Start at 1 every time Spectacle is launched" or something a bit less technical? Also, this control should probably only be enabled when one of the numbering tokens is actually being used.
I like how you've rephrased the instructions. Breaking it into two lines ends up being clearer and more concise.
It feels like we need better smarts to automatically guess the correct next number when Restart numbering with relaunch is unchecked, and the spinbox is a workaround for not having that. Thoughts?
I would agree with this. I really appreciate your perspective. Your questions are making me think that less is more with this, at least for now. I added a feature for a workflow that I imagined all myself.
How about I remove all the settings UI changes, the configuration manager changes, move the sequential numbering to the export manager, and let the sequential numbering reset to 1 with every relaunch to keep it simple? If users accidentally close the application, they can clean up the numbering manually in a file manager.
Sounds good! I think the two new lines of text that we've settled on to describe the feature are fine to add though.
I'd also support re-adding the automatically-continue-from-the-prior-number feature if you can code up a clever way for spectacle to figure it out from the on-disk files. If you can do that, then adding an option to turn that feature on or off would be appropriate.
Is there anything else you need from me to move forward on this? Not trying to rush you; take your time if you're busy!
Sorry. I didn't mean for this to seem to be stalled. I submitted a new patch with the result of our conversation above. It uses arc and is rebased on the upstream git repository as of today so hopefully we can avoid some of the merging issues of last. The new patch is D16304.
Great! Then this one should be Abandoned (you'll find that option under the Add Action... button above the comment box)