-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FR: easier debugging of configuration #92
Comments
@aspiers this package derives the parser from the major mode used, while Prettier normally uses heuristics mostly, but IIRC not exclusively, based on the file name. The idea was that Emacs might know better which parser to use, but I've come to regret that decision because it can lead to discrepancies. I'm guessing your case is a casualty. I've been planning to rewrite that part of the code, but haven't found the time yet. Somewhat related I also want to repurpose (parts of) https://github.com/editorconfig/editorconfig-emacs for setting indentation offsets etc. I can't remember immediately why but I think that's a prerequisite. As for your suggestion, I'm not convinced yet it's necessary. I'd like to first try to see if the removal of parser detection would help you. |
You could compare the parser used by the Prettier CLI vs that used by
I'm guessing they are different, if so try overriding the parser used by |
Thanks a lot for the quick responses! I've just stumbled across prettier/prettier#10876 which helped me figure out that the mismatch only occurs when the file is called
whereas According to that issue, this (undocumented!!) behaviour is intentional, but frustratingly, some stupid github bot has locked the issue for new comments. Any ideas how BTW, the fact that |
Aha! I was wondering if it might be Would you mind adding a bit of debug info to see why it's not working as intended? |
I think I found the issue: it's that BTW I've just filed prettier/prettier#11553. |
More detail: |
|
Yep - and after enabling, everything works perfectly. Phew! Thanks a lot for your help! Will you add those two to the default list? |
Ah, good find. Yeah I think those two are just an oversight. Happy to merge a PR right away, otherwise I'll try to get to it soon. |
BTW (and sorry for the spam, hopefully this is the last message) - going back to my original suggestion of making it easier to debug where config comes from, I think at very least something like the following would be helpful (untested): (defun prettier-current-parser ()
"Show the prettier parser selected for the current buffer."
(interactive)
(message "Prettier parser for current buffer is %s" (car (prettier--parsers)))) But even though that is a nice step in the right direction, it still doesn't answer my questions about whether prettier.el can handle multiple
While this is not perfect (e.g. it doesn't mention loading preferences from |
These two parsers were accidentally omitted from the default list. In particular, this broke the choice of parser for package.json files. So add them to the default list. This solves the original motivating factor for jscheid#92, but not jscheid#92 itself which is a request for enhanced debugging capabilities.
Note that the default modeline ("lighter") is already showing the parser -- except that it seems to be showing the wrong parser for some reason, but I'd rather fix that than add a new function.
Aside from the parser override, which as discussed I am already planning to remove, we're leaving all of this up to Prettier; meaning that there would be little benefit in having this package output the information if you can already ask the Prettier CLI for it. |
These two parsers were accidentally omitted from the default list. In particular, this broke the choice of parser for package.json files. So add them to the default list. This solves the original motivating factor for #92, but not #92 itself which is a request for enhanced debugging capabilities. Original commit by @aspiers, re-committed here in an attempt to fix a mysterious CI failure.
Actually, I checked and it works as intended. Does that help at all? |
My modeline is already too crowded and I don't think I have space to keep prettier info on there, to be honest. Since the logic already exists to figure out what to display to the user, why not allow two different ways to get it depending on preferences? Adding a tiny
IMHO there is more than a little benefit. For example (IIUC) there could be multiple local installations of prettier (e.g. for different nvm-installed versions of node), or other things which might go wrong in the integration between prettier and the elisp. In those cases, it's really useful to get the elisp's perspective, to check whether it aligns with the underlying prettier's perspective. The problem I encountered above (where prettier inferred So my suggestion is that users should be able to easily perform some of their own debugging even before having to learn the elisp internals of prettier.el. By "easily", I mean: a) without needing to know any elisp, and b) without having to figure out the exact CLI command for running prettier in a way which accurately mirrors how prettier.el is executing it. |
Fair enough. The info you seek is in buffer-local variable
I'm not so sure, it's the first bug report I've had in months and I can count on one hand the number of bug reports to do with a discrepancy between CLI and package; and then it's not even clear if the other reporters would have used the feature you're proposing rather than just raise a ticket here and let me figure it out. It took you and me all of 30 minutes to solve, and we both did something else during that time as well (in your case, file an issue with Prettier about the undocumented feature.) Maybe it would have taken less time if you had included the Building this feature, on the other hand, is going to take the better part of an evening and then someone will have to maintain it going forward. So I'm not convinced that this here was a good example of why we need this facility. In addition, this particular kind of problem, as well as the previous similar bug report (#88) won't happen anymore once I remove the custom parser detection. By the way, we let Prettier determine which In short, I'm not particularly keen on building this feature myself at this point in time. That being said, if you wanted to build it I'd be happy to lend you a hand and merge it, if it doesn't add all too many LOC that I will have to then maintain. |
Thanks very much for the reply and sorry for the delay in getting back to you. I've just experienced a completely new issue with prettier.el (this time with Solidity code rather than JSON), but it's put me back in the situation where I'm trying to debug the configuration. It seems relevant to post in this issue rather than a new one, because despite your very helpful tips above, I'm still unable to figure out what's going wrong. Maybe it will turn out to be a genuine bug in prettier.el rather than an issue with my configuration which needed debugging, in which case I'll happy retract the suggestion that this further reinforces the need for easier debugging of configuration. But currently it seems more likely that I'm doing something wrong than prettier.el has a bug ;-) The problem this time is that
Here is the censored https://gist.github.com/aspiers/d2f801669bf144f336eac5c6c7b9043a |
@aspiers the The That being said, the logic is to find the closest
|
@jscheid commented on September 27, 2021 7:07 AM:
This was definitely true originally, because I figured that out myself, and then did a global install of the solidity plugin to fix it. And you can see from this result which I previously posted above that the plugin install already had the desired effect:
Cool, thanks!
OK, so maybe the discrepancy lies somewhere in this logic. Which is exactly the kind of thing I was referring to above when I wrote:
So yes, surfacing this info somehow (in Also, even when it can't, is it really correct that |
Oh, do you just need to |
Wish it was that simple! Already tried multiple restarts... |
@aspiers I haven't forgotten about this, just been busy with other stuff but will try and get to this soon. |
Thanks a lot! I'm having quite a few issues with the mode at the moment and I can't figure out why, so some help would be super appreciated whenever you have time! |
@aspiers I've started working on this in #97: the first step is to add more info to Alas, the build fails with the same error that you ran into. I've investigated and it so happens that #66 fixes the problem. Since I've been meaning to do something with #66 anyway, I've rebased it and I'm going to dogfood it over the next few days. If there aren't any issues I'm planning to merge it and then pick up #97 again after. In the meantime you could help by also testing #66 (zipped tarball). Of course you can also use the WIP in #97 and see if it helps shed some light on the issue with solidity, but you'll have to compile/install it yourself. PS I too couldn't reproduce the error locally at first. Switching to a Docker setup loosely based on this let me reproduce it so that's another improvement I'm planning to make. |
Sounds fantastic, I will be glad to help testing! |
I have merged these two PRs into a temporary branch for testing, and I guess the batching is working fine (at least I didn't notice any problems) but I'm unsure how to benefit from the increased info, as I see it has only been added to the Javascript not to the elisp? I'm pretty desperate to figure out the configuration of the prettier process in question, because not only is it not correctly triggering for some Solidity files, it's also picking up the wrong config for some Javascript files, resulting in an endless war between emacs and the separate |
That sounds painful. I'm sure we'll get to the bottom of it. The increased info should appear in |
Make sure you run |
OK I finally got it working with a custom recipe! (use-package prettier
:straight (prettier :type git :flavor melpa
:files (:defaults "dist/*")
:branch "master" :host github
:repo "jscheid/prettier.el")) and now I see extra stuff in |
@aspiers yes would you mind opening a new issue? Could you mention which version of Yarn you're using and whether you're using PnP (I'm guessing not.) |
@aspiers still happy to take a look, but would be good if I could first recreate the issue here... for that I'd need to know your setup. |
I have a repo where if I run
yarn prettier -c foo.json
it formats it one way, and when I save it in emacs with thisprettier-mode
enabled, it formats it another way - and I can't figure out for the life of me why.I do appreciate that the
prettier
executable run viayarn
is different to the base64-encoded one which comes withprettier.el
(or at least it does in my case, when obtaining it from MELPA viastraight.el
, so maybe the difference is related to that, e.g. different prettier versions with different defaults? Or maybe they are somehow picking up different configurations? Although the latter seems unlikely since I only have one.prettierrc.yml
at the top of the repository in question.I've looked at the source to see if there's any way to find out the prettier config which the
prettier.el
long-running process is using, but there's nothing obvious. I'm not even sure whetherprettier.el
can simultaneously support multiple different configurations if your emacs has files from different projects open at once, each with different.prettierrc
settings?Describe the solution you'd like
M-x
command to show a) the prettier config just for the current file, and b) how that config was obtained. (I'm aware ofM-x prettier-info
, but that is a giant dump of the entire config which is so big, I have no idea where to start looking within that to see how the active options compare with the contents of the corresponding.prettierrc.yml
.*prettier (local)*
buffer (or in a separate buffer; I don't mind which), explaining the interactions with the server as they happen. That buffer remains entirely empty for me, which isn't very useful.Describe alternatives you've considered
I instrumented
prettier--sync-config
andprettier--load-config
with Edebug and stepped through them, but that just showed me some hugely complex data structure returned from the server, so it wasn't any use.The text was updated successfully, but these errors were encountered: