Skip to content
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

Updater for Windows 4.1 #317

Merged
merged 8 commits into from
Dec 25, 2017
Merged

Updater for Windows 4.1 #317

merged 8 commits into from
Dec 25, 2017

Conversation

claustromaniac
Copy link
Contributor

@claustromaniac claustromaniac commented Dec 24, 2017

Just very minor changes, mostly adding some missing information. including the changes suggested in #283 (comment).

@Thorin-Oakenpants I reply to you here, since I can't post comments on #283 because it says the conversation "has been locked and limited to collaborators" (strange, I know). Your work on the wiki looks good to me. I especially like what you said about hidden prefs in section 3.2, good call!

- added missing -unattended switch to the list of switches
- other minor changes
@Thorin-Oakenpants
Copy link
Contributor

conversation is unlocked now, forgot about that. I'll let earthlng handle the PR commit

the fix in question just removes the extra space in the version + date output (line 91)
IF !_line! GEQ 4 (GOTO exitloop)
IF !_line! EQU 1 (SET _name=%%H)
IF !_line! EQU 2 (SET _date=%%H)
IF !_line! EQU 3 (SET _version=%%G)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

( SET _version=%%G ) adds an unnecessary space after the version number, resulting in an output like:

ghacks user.js version 57 , 20 November 2017

@earthlng
Copy link
Contributor

Comments and _user.js.parrot lines are appended normally
One-line comments and ...

re: merge - why did you change this? multi-line comments are appended normally, too.

I also don't really like the parrot change. Users can add them themselves if + where they want them, IMO.

@claustromaniac
Copy link
Contributor Author

claustromaniac commented Dec 25, 2017

re: merge - why did you change this? multi-line comments are appended normally, too.

It's not technically accurate, because of the way it handles user_pref lines within multi-line comments. With this change it neither affirms nor denies that multi-line comments are appended. A full explanation would take many lines more. It would get redundant, too.

I also don't really like the parrot change. Users can add them themselves if + where they want them, IMO.

For me, either way is OK. It's up to you folks.

@Thorin-Oakenpants
Copy link
Contributor

Thorin-Oakenpants commented Dec 25, 2017

We can drop the parrot, I kinda like it though, because the last line in the user.js is a SUCCESS. And the final value should mirror that of ours as per picture (just the word success will do)

parrot

And if we kept it (parroting the last section), it should be a diff name because it's not ghacks user.js cannon ❗ However, to be technically correct, this value should be reset at the very start of the user.js, otherwise it might say success when it was never reached - so yeah, we should reuse the existing parrot or forget the whole thing

We should drop the whole parrot thing I think - soz, I thought it might be a good idea

Side question: I hope the parrot code (if we keep it) does not keep appending more parrots every update until you end up with dozens of parrots lines at the start and end (I assume you have this covered)

@Thorin-Oakenpants
Copy link
Contributor

Lets say we drop the parrot thing - If users add their own parrots, I thought the code did not append parrots (well at least not OUR parrot pref name) - right?

@claustromaniac
Copy link
Contributor Author

claustromaniac commented Dec 25, 2017

My reasoning (and I didn't actually test shit, this is just the way I understand things): Firefox will only read prefs.js and then user.js, and the updater just appends the overrides to user.js. Using the same parrot for that purpose should be ok, because when Firefox correctly reads the last parrot line, you know everything above that line was also read correctly. If something fails in the middle it will not get to the part with the overrides anyway. Isn't that how the parrot thingy is meant to work?

Side question...

The script DLs user.js, appends overrides, and then sticks this new parrot line at the end of the new modified user.js. The parrots can't multiply that way.

Lets say we drop the parrot thing - If users add their own parrots, I thought the code did not append parrots (well at least not OUR parrot pref name) - right?

The script appends parrot lines regardless of the mode (append or merge), which means that parrots are never merged, otherwise the merge algorithm would render them completely useless. I've been adding parrots to the overrides before I even wrote the first version of this script, so I think it is a good practice. I just don't mind if the script takes care of that part for me or not because it won't really save me much time by doing so anyway. I mean, my override files are only touched by me, so the parrots I add there won't go anywhere if I don't do anything.

@claustromaniac
Copy link
Contributor Author

Maybe it would be a good idea to just suggest that practice of adding manual parrots to the overrides in the wiki instead (if it's not there already). It is good advice and it can help people that don't want or can't use this script, too.

@Thorin-Oakenpants
Copy link
Contributor

Thorin-Oakenpants commented Dec 25, 2017

I'll let you and earthlng work it all out :)

The parrot works exactly as you said, but you would still need to add them at BOTH the start and end of the overrides appendage

// ghacks user.js
/* END: internal custom pref to test for syntax errors ***/
user_pref("_user.js.parrot", "SUCCESS: No no he's not dead, he's, he's restin'!");

// overrides appended by code
user_pref("_user.js.parrot", "you need something in here so the above SUCCESS is cleared");
   // ^^ I would use "overrides: syntax error"
...
...
user_pref("_user.js.parrot", "SUCCESS: No no he's not dead, he's, he's restin'!");

But I think we should just forget about the whole parrot thing - it was a silly idea, completely SILLY ... Monty Python SILLY

@earthlng
Copy link
Contributor

I thought the code did not append parrots

@Thorin-Oakenpants to help you understand how the script works, you should really give it a try. You don't have to run it in your real profile, you can just create a new empty folder somewhere, place the updater, a user.js and an override file in there and run the script with or without some of the cmd-line parameters.

@Thorin-Oakenpants
Copy link
Contributor

^^ true, but I have the utmost confidence in you two :) That and time factors .. or maybe I'm just making excuses

PS: closing wiki issue, will rejig numbers/order in section 2700 (persistent storage) and close that issue, and move to 58beta - if you're ok with that

@Thorin-Oakenpants
Copy link
Contributor

added a PRO TIP to 3.2 Applying Your Changes re re-using our parrot, and included the lines in the example - so we can drop the parrot thing in the updater.bat

@earthlng earthlng merged commit 1f15e28 into arkenfox:master Dec 25, 2017
@earthlng
Copy link
Contributor

got rid of the parrot stuff and merged it. Thanks

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

Successfully merging this pull request may close these issues.

3 participants