Add ability to disable all the features! by fregante · Pull Request #877 · refined-github/refined-github

fregante · 2017-12-15T13:47:23Z

Note: npm install needed.

Being developers, everything would ideally be an option
@bfred-it on 12 May 2016

So here we are... 😅

Doesn't clutter the interface with a thousand options
Doesn't clutter the code with a thousand conditions
Low maintenance cost

Add liability clause (i.e. if you disable something, good luck)
Add logging toggle to be able to find the feature names

The rawness of these options should discourage people from disabling stuff but they can if they try hard enough.

Fixes #779 #585 #515 #285 #193 #174 #131 #57 #10

Sadly doesn't fix any of the open change request since they're CSS-related.

Each feature has its own file; start/stop logic should not be outside. This keeps function names as intended and not `setup`

sindresorhus · 2017-12-15T17:37:24Z

Good idea. I think this is a good balance and now we have a place to refer most "option request" issues.

fregante · 2017-12-15T17:50:01Z

Cool! Give it a spin and let’s merge after we update the readme and extend the field description. Commits or suggestions welcome.

sindresorhus · 2017-12-15T17:58:50Z

source/libs/utils.js

- * The code looks weird but it's synchronous and fn is called without args.
 */
-export const safely = async fn => fn();
+export const enableFeature = async (fn, fileName) => {


Filename is most commonly used as one word, not file name.

sindresorhus · 2017-12-15T17:59:35Z

source/options.html

-			<input type="checkbox" name="hideStarsOwnRepos">
-			Hide other users starring/forking your repos
-		</label>
+		List the features to disable, by <a href="https://github.com/sindresorhus/refined-github/tree/master/source/features" target="_blank">file name</a>: <br>


file name => filename

sindresorhus · 2017-12-15T18:00:31Z

source/options.html

+		List the features to disable, by <a href="https://github.com/sindresorhus/refined-github/tree/master/source/features" target="_blank">file name</a>: <br>
+		<textarea name="disabledFeatures" rows="5" placeholder="For example:
+log-active-features
+hide-own-stars


It's weird to use literal newlines here. I would use \n escape instead.

\n doesn’t work but this might https://stackoverflow.com/a/983988

sindresorhus · 2017-12-15T18:02:40Z

source/libs/utils.js

-export const safely = async fn => fn();
+export const enableFeature = async (fn, fileName) => {
+	const {disabledFeatures} = await options;
+	const log = disabledFeatures.includes('log-active-features') ? () => {} : console.log;


Why have this as an option instead of just enabled by default when in dev mode?

Because it’s a user-facing feature also. One might want to see what’s enabled/disabled on a page. Maybe?

Could we have a checkbox to enable dev/debug mode instead? It doesn’t really feel like a feature in that sense.

What about enabling logging by default with console.group(), or even better console.groupCollapsed(), so that way console is not cluttered and the user still gets to see what features are enabled.

It doesn’t really feel like a feature in that sense

👍

I don't think extensions should clutter the console by default, even if it's just one item.

Let's go with a checkbox that is unchecked by default. It can be useful for other production debugging we might want later on too.

sindresorhus · 2017-12-15T18:04:07Z

source/background.js

 	},
 	migrations: [
+		options => {
+			if (options.hideStarsOwnRepos === true) {


Should be false

fregante · 2017-12-16T11:30:08Z

sindresorhus · 2017-12-16T12:40:27Z

source/background.js

 new OptionsSync().define({
 	defaults: {
-		hideStarsOwnRepos: true
+		disabledFeatures: 'log-active-features',


Shouldn't this be empty now?

sindresorhus · 2017-12-16T12:42:01Z

source/background.js

 	defaults: {
-		hideStarsOwnRepos: true
+		disabledFeatures: 'log-active-features',
+		logFeatures: false


I was thinking we make this more generic so we could use it for other production logging in the future. This locks it into just being logging of features.

Done. When that happens the feature can be developed further.

sindresorhus · 2017-12-17T10:05:13Z

Awesome! Some people are gonna love this.

fregante · 2017-12-17T10:41:00Z

Yep! Can you update the readme?

sindresorhus · 2017-12-17T10:42:10Z

Sure

Disable all the features!

fd7749d

fregante added change request enhancement under discussion labels Dec 15, 2017

fregante requested review from hkdobrev, jgierer12 and sindresorhus December 15, 2017 14:02

fregante added 5 commits December 15, 2017 22:05

Typo

712a3d2

Simplify option migration

acaa860

Avoid handling setup/destruction outside the modules

2d1ce19

Each feature has its own file; start/stop logic should not be outside. This keeps function names as intended and not `setup`

List features by file name instead of function name

1d31a5d

Enable/disable logging via disabledFeatures

fda132f

sindresorhus changed the title ~~Disable all the features!~~ Add ability to disable all the features! Dec 15, 2017

fregante added the needs readme update label Dec 15, 2017

sindresorhus requested changes Dec 15, 2017

View reviewed changes

fregante added 2 commits December 16, 2017 19:29

Apply requested changes

ed09a49

Options: Add notice and shorten existing copy

f7e70b8

Merge branch 'master' into option-hell

30f80cf

sindresorhus reviewed Dec 16, 2017

View reviewed changes

Generalize logging

d696de0

sindresorhus approved these changes Dec 17, 2017

View reviewed changes

sindresorhus merged commit f4fca5f into master Dec 17, 2017

sindresorhus deleted the option-hell branch December 17, 2017 10:05

sindresorhus mentioned this pull request Dec 17, 2017

Document the option for disabling individual features #881

Merged

fregante mentioned this pull request Dec 19, 2017

Keyboard shortcut conflicts: Control-Shift-U #673

Closed

fregante added the meta Related to Refined GitHub itself label Dec 29, 2017

Uh oh!

Conversation

fregante commented Dec 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sindresorhus commented Dec 15, 2017

Uh oh!

fregante commented Dec 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fregante Dec 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fregante commented Dec 16, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sindresorhus commented Dec 17, 2017

Uh oh!

fregante commented Dec 17, 2017

Uh oh!

sindresorhus commented Dec 17, 2017

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

fregante commented Dec 15, 2017 •

edited

Loading

fregante commented Dec 15, 2017 •

edited

Loading

fregante Dec 16, 2017 •

edited

Loading