Improve error handling using statusbar item & output channel

[RobinMalfait]

Hello!

I was the author of another prettier extension which I deprecated for this one.

In the package I wrote I had some nice indications if your code was prettified correctly or not. & . Now I know that not everybody wants it so I added an option to enable/disable this feature.

Another thing is, the current error handling is quite annoying in my opinion, you get a popup message with an error and you can't really save the file. For example when you are editing the settings.json file from VSCode, you will get errors because it contains invalid JSON, the // comments. What this PR enables is that we execute the formatting in a save way and if an error occurs we return the defaultValue (which is the current text), this way we can actually edit files with invalid JSON such as settings.json.

Last but not least, we still want to view our errors. For this very reason I added an output channel where I write the errors when something happens. If we press the statusbar item we also open the output channel.

Errors look like this:

[CiGit]

Thanks. I've no time to test it right now but definitely will 😄 I will asked it here: I love prettier, I hate options. Is that option really necessary? IMO the status bar indicator could be even smaller :

P: ❌ P: ✔️ with a tooltip

Is it possible to insert clickable links in those outputs? In your screenshot I'd like to click on the error and being brought to the location. Like Show currently. If not, I would add the file which produced the error in the output text. Edit: Ok, you just did that last point

[RobinMalfait]

@CiGit I hate options too, I will delete the option for now, and if people request it we can re-enable it if necessary. I don't really like the shorter version of P: ... because if you have more extensions that do that it can get messy in that bar. Prettier: ... is short enough in my opinion.

[CiGit]

BTW:

we execute the formatting in a save way and if an error occurs we return the defaultValue (which is the current text), this way we can actually edit files with invalid JSON such as settings.json

It was already possible. Failing on format still saves it, with a ugly error message ;-)

---

I've played with it a little. Now we do not see errors anymore. That's good. Note: I've searched for Prettier:... on the right side first. We see only the last error/all good in the output. Meaning if I format multiple files, I get only the last output, silencing the rest. outputChannel.clear();. What I had in mind was using it like an append only console. The user can still clear it.

[RobinMalfait]

@CiGit oh cool, let me update the output a little bit.

[RobinMalfait]

In the last commit, I made it possible to append to the output channel. It was quite a mess so I added some titles, what do you think?

A title looks like this (so that there is a nice "gap" between errors):

[CiGit]

I like it. Maybe don't output "All good" that's just noise.

[RobinMalfait]

Done! :D

[CiGit]

And maybe remove the '\n' between ---------- and the error. This will group timestamp with the corresponding message.

I'll review the code a little latter. In the meantime can you rebase and squash all those commits, except the one which removes the option (if we want to revert once)

[RobinMalfait]

Sure, I'll squash them together.

[CiGit]

We now have the same problem with the status icon, it shows only the last run ( ❌ ✔️ )

[RobinMalfait]

Regarding to your last comment

We now have the same problem with the status icon, it shows only the last run ( ❌ ✔️ )

How would you handle this? For error output it is easy, we can just append the the outputChannel. For this, I don't want to do management of files I guess? Or would you display the information based on the current active file? When you switch tabs to another file, display the correct information there?

[CiGit]

Yeah this is something to think about... It feels just weird to have ✔️ with errors. I've some ideas:

• show the ❌ once an error occurred. Remove it on clear if possible.
• Have an error marker on each file. And show it depending on the focus. Something harder to do.
• Show nothing...

This could be an other PR tho. I find this one is a good enhancement. Let me know your thought. 🛌

[RobinMalfait]

I also think that it should be a future improvement and in a separate PR. I like this one the most:

Have an error marker on each file. And show it depending on the focus. Something harder to do.

Because in the first option you have to keep track of all the items. And last option is not ideal because I love the ❌ & ✔️

[CiGit]

Seems we misunderstood ourself.

Because in the first option you have to keep track of all the items. That's not what I meant. Error -> :x: -> outputChannel. on("clear") -> :heavy_check_mark:

In the second option, you will have to keep a list of files which have errors.

New idea: Don't use status bar item. Open the output channel on first error or provide an action "show prettier errors"

[RobinMalfait]

I think that opening the output channel on error will be very annoying, especially when you have the editor.formatOnType set to true.

[CiGit]

It will only open the first time ie on create

[RobinMalfait]

Okay another proposal:

What if we do a dry_run everytime we switch to an open file. If an error occurs show Prettier: X else Prettier: V but don't really save it?

It will only open the first time ie on create

That's also an option. Which do you prefer?

[CiGit]

I don't think your proposal fits prettier's role. IMO that's a linter's role.

[RobinMalfait]

Last changes include:

• Show warning once per workspace
• Open output channel on first error per workspace

[CiGit]

Seems to me: Open output channel on first error per workspace

I could nit-pick, but LGTM. We can still change things later on. Great job on this first iteration :-)

Will let it open for some potential reviews. Will merge it this evening and release. Please squash :-)

[RobinMalfait]

I'll squash everything together. If we ever need to re-add the option to show/hide the statusbar it's a small change.

[CiGit]

Thanks a lot for your work. It has been released! If you agree, we should ask @esbenp to give you the same status I have. You seemed involved to me.

[RobinMalfait]

@CiGit thank you! I love prettier and loved building my extension. But there is no need to maintain multiple extensions. 1 good extension is key.

With status you mean collaborator and be allowed to merge PR's? I'll try my best to help wherever I can to improve the package even more :)

[CiGit]

Yes Collaborator. This means having the right to merge PR, push to master, close issues ........... and maybe even publish if we can have this from @esbenp (who vanished). I feel so lonely here :-)

[RobinMalfait]

Hehe, it might also be an option to move the prettier-vscode repo to the prettier namespace? That way we don't have to spam @esbenp so much :p and/or there will be more issues & PR's?

[CiGit]

He is the one who can move that repo. #41 There are always issues, PR. There is 1 PR awaiting. And there is more to do here.