sindresorhus / atom-editorconfig

Helps developers maintain consistent coding styles between different editors
https://atom.io/packages/editorconfig
MIT License
812 stars 80 forks source link

allow ability to disable/persist notifications #213

Closed heavyk closed 5 years ago

heavyk commented 6 years ago

question: I don't see any reason why persistNotifications should be true. the user has to manually click on the notification to remove it. is this desired default behaviour?

EDIT: also the tests are totally broken. I get from 14-19 failures each try. also, I don't know how I would go about testing this configuration, nor do I think it's really necessary


To make the maintainer's life easier, I...

florianb commented 6 years ago

Hi @heavyk - thank you for your contribution.

The notifications are persisted since they may contain information for debugging purposes and the user shouldn't miss any information accidentially.

We also try not to use atom-configurations. The package should just work.

I guess you're trying to fix a bunch of files and i understand that this might be an annoying task using editorconfig. I'd like to suggest you using a third party tool for batch-processing, like https://www.npmjs.com/package/editorconfig-tools.

I won't merge this PR since i don't see the necessity to introduce configuration options. However - it is debatable to make fix-messages disappearing automatically.

heavyk commented 6 years ago

in this issue: https://github.com/sindresorhus/atom-editorconfig/issues/173 fix on save function is given. I added it as an init script to run the editorconfig fixer onSave and the notifications were getting out of hand.

the thing about the configuration is just to allow the ability to disable a UX notification. no functionality has been changed.

I can see how if you ran the fix command and no notification was made, the user may believe that nothing happened. however, if it's done onSave, then the notifications aren't really necessary.

in that same issue, you suggest that instead whenever someone saves a file, do a check and alert the user if the saved file does not conform. I could add that as well. it does seem like a good idea, but I was under the impression that if a project has an editorconfig, then the project does already conform to those rules, so no user interaction should be needed.

let's say I implement your idea, I can see the value in the ability to opt-in to fix it automatically and instead, I'm just shown a non-persistent notification saying a malformed file has been fixed

florianb commented 6 years ago

I understand - and as more as i think about it, i'd entirely agree on automatically dismiss the notifications for automatically converted files.

However i see the following points to be solved for an automatic fixing of files f.e. `onSave´:

I could also imagine to detect malformed files and notify the user about it and offer a command for fixing the entire project to handle a mass-import. It is also possible to detect a malformed file and then offer a link in the notification to immediately fix the file. Or offer the link for big files and fix it automatically in smaller ones.

heavyk commented 6 years ago

so, since I do not think it will be easy to performantly detect a malformed file, I think there should only be two things done:

this should satisfy 98% of use-cases and anyg remaining cases can be controlled by the user's scripts

Alhadis commented 5 years ago

the user has to manually click on the notification to remove it

Just an FYI: You can hit ESC to dismiss all notifications at once (I went for a long time without even knowing this was possible). However, you're right in that the notification noise should be adjustable — if you run fix and no changes were needed/done, there's no point having a popup appear reminding you that life is wonderful and there's really nothing to worry about.

So while I'm sympathetic with your frustration, I don't want to bloat this package with a bunch of fine-grained verbosity controls filtering how much information gets displayed, for how long, and under what circumstances. If you're running the fix command often enough that even hitting ESC to dismiss them becomes laborious, then perhaps a fix-quietly command would be preferable. Something that's a "Don't bug me" version of fix, with the consequence that any important information (such as unexpected errors) would remain unnoticed.

Remember, the notification system is also used for displaying status-reports, something users expect to see (in detail, I might add). Because this package is using the notification system for displaying both important and spammy feedback, there's no solution that would be obvious and beneficial to everybody.

Alhadis commented 5 years ago

BTW, if this ever happens again with another package, you can tell it to completely shut the fuck up by adding this to your ~/.atom/init.js file (or loading it from a file somewhere nearby):

Then, to block notifications generated by the editor:log-cursor-scope command (or in this instance, EditorConfig:fix-file, you'd use either of the following:

blockNotificationByCommand("editor:log-cursor-scope");
blockNotificationByPattern(/Scopes at Cursor/);;

Feel free to use and share. Plenty more hacks in my Atom setup that you can pillage that'll show how much a grouchy, intolerant bastard I really am. 😀

heavyk commented 4 years ago

wow dude, thanks for linking your atom config. super inspiring :)