pulsar-edit
Constraints in configuration settings should be explained in the UI
Thanks in advance for your bug report!
- [X] Have you reproduced issue in safe mode?
- [X] Have you used the debugging guide to try to resolve the issue?
- [X] Have you checked our FAQs to make sure your question isn't answered there?
- [X] Have you checked to make sure your issue does not already exist?
- [X] Have you checked you are on the latest release of Pulsar?
What happened?
This is a bug that is present in Atom from before the sunset... I found it yesterday. Earlier today I found out about Pulsar and installed the latest Windows version. The bug is still present. The issue is with setting your "Max Screen Line Length" is broken (mostly). See my description below for better details and info or the conversation I had with @Daeraxa on Discord here.
Pulsar version
1.113.0
Which OS does this happen on?
🪟 Windows
OS details
Windows 11 Pro Beta v23H2 (22635.3130)
Which CPU architecture are you running this on?
x86_64/AMD64
What steps are needed to reproduce this?
Open Pulsar, go to Settings -> Editor, scroll down and set your "Preferred Line Length" to something, ex. I used 120 (Not sure this is needed though). Now, scroll up to "Max Screen Line Length" and try to set it to 150 (or anything below 500) and it will delete the numbers as you enter them. You can spam numbers to make the number above 500 to 'stick' but you have to type it quickly or it deletes as you type.
If you go to File -> Config... to open the config.cson file and try to manually add the setting for this (it's either missing or set to whatever you tricked it into above).
maxScreenLineLength: 9999999
If you try to set it here to say 150 (anything below 500), save the file and that entire line will auto-delete from your settings file.
Additional Information:
I haven't tested Pulsar for macOS yet, but I'm like 90% sure this bug is present in Atom on Mac, so I assume it's the same there. I can test it tomorrow on macOS Ventura 13.6.3 if the info will be helpful.
As mentioned in the thread, if there is a valid reason for values <500 to be disallowed then that needs to be part of the description or it should error and flag it to the user.
Is this issue about the UI — which, naturally, should give you more helpful feedback than silently deleting numbers — or is it asking why the setting enforces a minimum of 500?
Here’s the original PR. From what I gather:
- Atom didn't do well with being asked to render extremely long lines, performance-wise, in situations where people had disabled soft wrap in their editors
- Breaking long buffer lines into separate screen lines made it so that minified files — which would apparently lock up the editor if the lines were long enough — were at least manageable, so they needed to pick a length at which to say “sorry, we tried not to soft-wrap the line, but we had to do this so the editor doesn’t lock up”
- It sounds like they arrived at 500 because they wanted something high enough that it couldn't possibly apply to a line that was legitimately that long in a source code file
- This was a feature designed to do something that would annoy the user by violating their stated soft-wrap preference, so I don’t think anyone ever argued until now that they should to be able to increase the chances of it occurring
- So the goal wasn't necessarily to offer users a new way to sculpt their editor environment — it was that 500 used to be hard-coded just as a way of managing an implementation detail of the editor, and some people argued that it should be able to be set higher if the user was willing to accept the potential trade-offs
I can’t think of a particular reason why someone shouldn’t be able to set this value to something below 500… but if you want to set it to 150, why not just set preferredLineLength to 150? What’s the problem you’re trying to solve?
I can’t say that I’ve ever thought about this setting at all, but if I could snap my fingers and make it so, I’d want to give maxScreenLineLength much less prominent treatment than preferredLineLength in the UI just so people don’t mix them up. I imagine that weird things could happen if the former setting is allowed to be much lower than the latter. The only thing I can say in defense of the minimum is that it mostly prevents such a thing from happening.
Anyway, let me get the inarguable out of the way: when a setting has a constraint like this, it should be indicated in the UI somehow, so that people understand why the value they type in isn’t being accepted. In the short term, the description should certainly be edited to mention it, but the UI should find a way to indicate it implicitly even when it’s not in the description.
Similarly, I don’t love that a configuration line gets silently deleted from your config file when it fails a constraint, but I’m not 100% sure of how the editor should let you know. I think when it reacts to your manual editing of the file, then some sort of notification is warranted, as long as that we can guarantee that that won’t ever happen when a setting is changed to something invalid by a package or someone’s init file or something else that’s automated. I don’t want even a small risk of someone getting notification spam because a community package is doing something bad and the user has no idea how to make it stop.
Honestly, my new medication kind of makes my thoughts a bit cloudy so I can't remember the exact reason I wanted to set the two values different (120 and 150) but I was given a piece of code from a friend who asked what was wrong with it... their code had no line breaks and I was sick of scrolling to the right. :) But in the end, this isn't something that 'breaks' the software for me and it doesn't bother me, I just figured I'd point it out since it appears to have been in the code for a long time unnoticed. As both of you have stated, it's not a big deal that you can't use a value under 500, but it'd be nice if UI explained why you can't do that and as you mentioned, it was a little jarring that if you set it manually and save (not close) the file you can watch the software just outright delete that setting from the file.
Gotcha.
I’m happy to keep this bug open as a reminder that we need to have a friendlier settings UI.
I would add there is additional undesirable behaviour here in general. If the number has to be greater than 500 you still have to type it in. This is where that "auto delete" is really problematic both in general behaviour and accessibility. If I wanted to up that to 1000 I would need to type all 4 numbers in really quite rapid succession to avoid it deleting them. This is a pretty awful bit of UX.
Definitely — it should assert itself when the field loses focus, or if the user presses Return, but not instantaneously as the value changes. That’s an obvious bug.