UI for editing @include/@exclude #1362

Closed
arantius opened this Issue Jun 3, 2011 · 11 comments

Comments

Projects
None yet
5 participants
@arantius
Collaborator

arantius commented Jun 3, 2011

The feedback has grown to overwhelming levels:

Of course feedback is not universally negative:

(P.S. I hate how hard AMO makes it to monitor this stuff.)

@Tobbe

This comment has been minimized.

Show comment
Hide comment
@Tobbe

Tobbe Jun 3, 2011

Contributor

I can only speak for myself, but this is what I would want:
A GUI that lets me add/edit/delete included/excluded sites that syncs with the actual script file. So no matter where I make the changes both the GUI and the script file is up to date and shows the actual pages the script will run on.

Contributor

Tobbe commented Jun 3, 2011

I can only speak for myself, but this is what I would want:
A GUI that lets me add/edit/delete included/excluded sites that syncs with the actual script file. So no matter where I make the changes both the GUI and the script file is up to date and shows the actual pages the script will run on.

@arantius

This comment has been minimized.

Show comment
Hide comment
@arantius

arantius Jun 3, 2011

Collaborator

So, by implication, you're asking for a feature whereby Greasemonkey itself alters the .user.js file you installed (to give it different @include / @exclude lines), right?

Collaborator

arantius commented Jun 3, 2011

So, by implication, you're asking for a feature whereby Greasemonkey itself alters the .user.js file you installed (to give it different @include / @exclude lines), right?

@Tobbe

This comment has been minimized.

Show comment
Hide comment
@Tobbe

Tobbe Jun 3, 2011

Contributor

Yes

Den3 jun 2011 17:29, skrevarantius
reply@reply.github.com:

So, by implication, you're asking for a feature whereby Greasemonkey
itself alters the .user.js file you installed (to give it different
@include / @exclude lines), right?

Reply to this email directly or view it on GitHub:

#1362 (comment)

Contributor

Tobbe commented Jun 3, 2011

Yes

Den3 jun 2011 17:29, skrevarantius
reply@reply.github.com:

So, by implication, you're asking for a feature whereby Greasemonkey
itself alters the .user.js file you installed (to give it different
@include / @exclude lines), right?

Reply to this email directly or view it on GitHub:

#1362 (comment)

@Martii

This comment has been minimized.

Show comment
Hide comment
@Martii

Martii Jun 3, 2011

Contributor

If I may make a suggestion... perhaps an API to allow a different add-on to interface with GM could be feasible and keep it separate from the GM core addon... similar in nature to Greasefire. Just a thought.

Contributor

Martii commented Jun 3, 2011

If I may make a suggestion... perhaps an API to allow a different add-on to interface with GM could be feasible and keep it separate from the GM core addon... similar in nature to Greasefire. Just a thought.

@arantius

This comment has been minimized.

Show comment
Hide comment
@arantius

arantius Jun 3, 2011

Collaborator

So as far as 'clude rules go, the pre-0.9 manage dialog had:

  1. Prominent display of 'clude rules, in selectable list boxes.
  2. A set of add/edit/remove (the latter two acting on the selected line) buttons for each.
  3. The add dialog would default to http://www.example.com/* (for whatever location value from the active browser tab which comes back from .getMostRecentWindow("navigator:browser")).

That's it, right?

Collaborator

arantius commented Jun 3, 2011

So as far as 'clude rules go, the pre-0.9 manage dialog had:

  1. Prominent display of 'clude rules, in selectable list boxes.
  2. A set of add/edit/remove (the latter two acting on the selected line) buttons for each.
  3. The add dialog would default to http://www.example.com/* (for whatever location value from the active browser tab which comes back from .getMostRecentWindow("navigator:browser")).

That's it, right?

@kwah

This comment has been minimized.

Show comment
Hide comment
@kwah

kwah Jun 4, 2011

I'm trying to balance out the different approaches to editing the file directly.. No doubt I'll not have thought through them all.

On the one hand hunting and picking out individual lines to edit/remove on the face of it seems much more straightforward, but deciding how to handle duplicate includes/excludes and the creation of a race condition (read/edit/write the script before another program edits the file) makes this my least favoured option.

I suggest that if editing the metadata block directly becomes an option that rather than hunt and pick out which lines need to be edited, the metadata information is instead read into a javascript object, the original metadata block is removed and then the new metadata block in its entirety is instead written to the script itself.

The only alternative I can think of is to display the isolated script metadata and provide 'Edit/Remove this line' and 'Insert line here' functionality - Arantius, perhaps this is a more elaborate (not being restricted to include/exclude) version of what you have in mind when you say "acting on the selected line"? - with a prominent display if duplicated include/exclude rules are found.

kwah commented Jun 4, 2011

I'm trying to balance out the different approaches to editing the file directly.. No doubt I'll not have thought through them all.

On the one hand hunting and picking out individual lines to edit/remove on the face of it seems much more straightforward, but deciding how to handle duplicate includes/excludes and the creation of a race condition (read/edit/write the script before another program edits the file) makes this my least favoured option.

I suggest that if editing the metadata block directly becomes an option that rather than hunt and pick out which lines need to be edited, the metadata information is instead read into a javascript object, the original metadata block is removed and then the new metadata block in its entirety is instead written to the script itself.

The only alternative I can think of is to display the isolated script metadata and provide 'Edit/Remove this line' and 'Insert line here' functionality - Arantius, perhaps this is a more elaborate (not being restricted to include/exclude) version of what you have in mind when you say "acting on the selected line"? - with a prominent display if duplicated include/exclude rules are found.

@kwah

This comment has been minimized.

Show comment
Hide comment
@kwah

kwah Jun 4, 2011

At the risk of jumping the gun / over-thinking this, on a practical note, perhaps this work-flow could work:

  1. Read the metadata block as usual into a javascript object, but take note of the starting and ending line numbers

    1a. If duplicates occur, prominently display a note to this effect stating that duplicates will be removed and that the order of the script's metadata may be rearranged

  2. When saving the file, cache it and do the following:

    2a. Write all lines upto and including the // ==UserScript== line from the original script
    --> nb: see http://wiki.greasespot.net/Knowing_Your_Own_Metadata for a legitimate reason for this being necessary
    2b. Then write the metadata block from the created javascript object
    --> the layout for this will need to be decided.. perhaps this order: @name, @namespace, @version, @description, @copyright, @license, blank line, @includes, @excludes, blank line, @require, @resource, @icon, blank line, @unwrap, blank line, @(all others, eg history), blank line, @uso:(values)
    2c. Skip the existing metadata block and write the // ==/Userscript== line & all remaining lines of the script

  3. Perform a diff to check that changes only occur within the range that we found the metadata in (1)

    3a. If affirmative, overwrite the existing script
    3b. Else display a warning message that the script may have changed whilst being processed, display the diff(?), and provide the option to reload the script's metadata or write it anyway

kwah commented Jun 4, 2011

At the risk of jumping the gun / over-thinking this, on a practical note, perhaps this work-flow could work:

  1. Read the metadata block as usual into a javascript object, but take note of the starting and ending line numbers

    1a. If duplicates occur, prominently display a note to this effect stating that duplicates will be removed and that the order of the script's metadata may be rearranged

  2. When saving the file, cache it and do the following:

    2a. Write all lines upto and including the // ==UserScript== line from the original script
    --> nb: see http://wiki.greasespot.net/Knowing_Your_Own_Metadata for a legitimate reason for this being necessary
    2b. Then write the metadata block from the created javascript object
    --> the layout for this will need to be decided.. perhaps this order: @name, @namespace, @version, @description, @copyright, @license, blank line, @includes, @excludes, blank line, @require, @resource, @icon, blank line, @unwrap, blank line, @(all others, eg history), blank line, @uso:(values)
    2c. Skip the existing metadata block and write the // ==/Userscript== line & all remaining lines of the script

  3. Perform a diff to check that changes only occur within the range that we found the metadata in (1)

    3a. If affirmative, overwrite the existing script
    3b. Else display a warning message that the script may have changed whilst being processed, display the diff(?), and provide the option to reload the script's metadata or write it anyway

@arantius

This comment has been minimized.

Show comment
Hide comment
@arantius

arantius Jun 6, 2011

Collaborator

On the one hand hunting and picking out individual lines to edit/remove on the face of it seems much more straightforward

At this point, I can find no plan more reasonable than guaranteeing that, if we are in fact modifying these files, that we make as little change as possible. No other lines should be changed or moved besides those which have to, based on the user's selection.

but deciding how to handle duplicate includes/excludes and the creation of a race condition (read/edit/write the script before another program edits the file) makes this my least favoured option.

Duplicates? Let them be, as they would if the original script had them. No harm but a tiny little runtime sag.

The whole reason changing the file on disk worries me is the "another program edits the file" bit. If I, a user:

  1. Open the script in an editor (and possibly save changes)
  2. Implicitly edit the file on disk via this new UI
  3. (Possibly change and then) save the file, in the editor from step 1

I erase the changes in step 2. As long as we're treating the script as the one true source of this data, there's not a lot to do about it, though.

I suggest that if editing the metadata block directly becomes an option that rather than hunt and pick out which lines need to be edited, the metadata information is instead read into a javascript object, the original metadata block is removed and then the new metadata block in its entirety is instead written to the script itself.

Strongly disagree. First, the possibility for bugs there is huge. Second, I guarantee someone out there cares about the ordering and/or spacing of these lines and will get angry when we (unnecessarily) change them. There's a lot of scripts out there, doing a lot of things we never expected. We have to be extremely defensive, so as to not do something to break those scripts.

The only alternative I can think of is to display the isolated script metadata and provide 'Edit/Remove this line' and 'Insert line here' functionality

Perhaps. But the vast majority of complaints we have are an isolated "it's too hard" (or just impossible) with no justification for why it is hard. Would this specifically mollify these users? I can't answer, but I can guess that it would not.

Arantius, perhaps this is a more elaborate (not being restricted to include/exclude) version of what you have in mind when you say "acting on the selected line"? - with a prominent display if duplicated include/exclude rules are found.

No, I just mean that, in the old UI, you could just click a line and then 'remove' and it was gone, that's it (or edit to change it). Very simple, very little thinking involved. And you didn't have to figure out which line it was really, it was isolated to the point where it was obvious. No thinking or worrying about breaking some other (@name @Version @whatever) stuff.

Collaborator

arantius commented Jun 6, 2011

On the one hand hunting and picking out individual lines to edit/remove on the face of it seems much more straightforward

At this point, I can find no plan more reasonable than guaranteeing that, if we are in fact modifying these files, that we make as little change as possible. No other lines should be changed or moved besides those which have to, based on the user's selection.

but deciding how to handle duplicate includes/excludes and the creation of a race condition (read/edit/write the script before another program edits the file) makes this my least favoured option.

Duplicates? Let them be, as they would if the original script had them. No harm but a tiny little runtime sag.

The whole reason changing the file on disk worries me is the "another program edits the file" bit. If I, a user:

  1. Open the script in an editor (and possibly save changes)
  2. Implicitly edit the file on disk via this new UI
  3. (Possibly change and then) save the file, in the editor from step 1

I erase the changes in step 2. As long as we're treating the script as the one true source of this data, there's not a lot to do about it, though.

I suggest that if editing the metadata block directly becomes an option that rather than hunt and pick out which lines need to be edited, the metadata information is instead read into a javascript object, the original metadata block is removed and then the new metadata block in its entirety is instead written to the script itself.

Strongly disagree. First, the possibility for bugs there is huge. Second, I guarantee someone out there cares about the ordering and/or spacing of these lines and will get angry when we (unnecessarily) change them. There's a lot of scripts out there, doing a lot of things we never expected. We have to be extremely defensive, so as to not do something to break those scripts.

The only alternative I can think of is to display the isolated script metadata and provide 'Edit/Remove this line' and 'Insert line here' functionality

Perhaps. But the vast majority of complaints we have are an isolated "it's too hard" (or just impossible) with no justification for why it is hard. Would this specifically mollify these users? I can't answer, but I can guess that it would not.

Arantius, perhaps this is a more elaborate (not being restricted to include/exclude) version of what you have in mind when you say "acting on the selected line"? - with a prominent display if duplicated include/exclude rules are found.

No, I just mean that, in the old UI, you could just click a line and then 'remove' and it was gone, that's it (or edit to change it). Very simple, very little thinking involved. And you didn't have to figure out which line it was really, it was isolated to the point where it was obvious. No thinking or worrying about breaking some other (@name @Version @whatever) stuff.

@0rt

This comment has been minimized.

Show comment
Hide comment
@0rt

0rt Jun 11, 2011

It there any chance bring back the UI in 0.8., please.
I don't know how other people think of FF4's new addon-manager, "it sucks" if you ask me.

Can't believe GM just gave up the well-constructed UI, to make that worse, replaced it with a neither nice looking nor providing the basic function like @include & @enclude UI.

I miss the old UI, ORZ

0rt commented Jun 11, 2011

It there any chance bring back the UI in 0.8., please.
I don't know how other people think of FF4's new addon-manager, "it sucks" if you ask me.

Can't believe GM just gave up the well-constructed UI, to make that worse, replaced it with a neither nice looking nor providing the basic function like @include & @enclude UI.

I miss the old UI, ORZ

@arantius

This comment has been minimized.

Show comment
Hide comment
@arantius

arantius Aug 8, 2011

Collaborator

arantius/greasemonkey@master...clude-ui

Those commits above are here.

Collaborator

arantius commented Aug 8, 2011

arantius/greasemonkey@master...clude-ui

Those commits above are here.

arantius added a commit to arantius/greasemonkey that referenced this issue Aug 9, 2011

arantius added a commit to arantius/greasemonkey that referenced this issue Aug 9, 2011

@arantius

This comment has been minimized.

Show comment
Hide comment
@arantius

arantius Aug 9, 2011

Collaborator

Merged! Will be in next nightly. Please test!

Collaborator

arantius commented Aug 9, 2011

Merged! Will be in next nightly. Please test!

@arantius arantius closed this Aug 9, 2011

arantius added a commit to arantius/greasemonkey that referenced this issue Aug 10, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment