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 v4.3 #341

Merged
merged 6 commits into from
Jan 18, 2018
Merged

Updater for Windows v4.3 #341

merged 6 commits into from
Jan 18, 2018

Conversation

claustromaniac
Copy link
Contributor

@claustromaniac claustromaniac commented Jan 16, 2018

I'm being quoted to introduce something,
but I have no idea what it is and certainly
don't endorse it. — Randall Munroe

(sorry, I couldn't help myself)

I believe I gave you folks a decent enough rest from my incessant pull requests, so here I come again.

Just think that the more I bother you now, the less I'll bother you in the future. I mean, if these scripts someday become sentient beings capable of maintaining themselves, and if they are willing to, that will most likely be the last time you'll see me around.

Yes, @earthlng. I know I can't make sentient batch files. It was a joke this time.

Changes:

  • The script doesn't touch the user.js file until it really has to.
  • The merge function is a bit smarter parsing files, at no significant cost. See examples below.
  • Fixed a minor issue with the version check.
  • Minor syntactic changes here and there.

Additions:

  • New -multiBackups argument. I personally intend to use it to compare files and quickly review changes. Ditched for -singleBackup argument.
Merge function examples (click to expand)

user.js contains:

user_pref("first", "aaaaa");
user_pref("second", "aaaaa");

user-overrides.js contains:

user_pref(	"first"  , "bbbbb");
user_pref(	"first"  , "ccccc");
user_pref('first', 'ddddd');
user_pref('first', 'eeeee');
user_pref("second",
 "bbbbb");
user_pref("second", "ccccc");
user_pref("third",
 "aaaaa");
user_pref("third", "bbbbb");

Result without these changes:

user_pref("first", "aaaaa");
user_pref("second", "ccccc");
user_pref(	"first"  , "ccccc");
user_pref('first', 'eeeee');
 "bbbbb");
user_pref("third", "bbbbb");
 "aaaaa");

Result with these changes:

user_pref("first", 'eeeee');
user_pref("second", "ccccc");
user_pref("second",
 "bbbbb");
user_pref("third",
 "aaaaa");
user_pref("third", "bbbbb");

P.S.: Forgive the copy-pasted commit description. I should've edited it at the very least. Oh well.. whatevs.

Changes:
	-The script doesn't touch the user.js file until it really has to.
	-The merge function is a bit smarter parsing files, at no significant cost. See examples below.
	-Minor syntactic changes here and there.
Additions:
	-New -multiBackups argument. I personally intend to use it to compare files and quickly review changes.
'IF !_line! GEQ 4 (' is not reliable.
@earthlng
Copy link
Contributor

Hi! meow meow. No idea who Earthing is but I'm sure (s)he appreciates being summoned to witness this :)

oh boy, lots of changes again. Will probably take a while until I have time to look at this in detail.
Any reason why we should keep singlebackup? I would prefer multibackup as the default, maybe with an optional -singlebackup.

@claustromaniac
Copy link
Contributor Author

claustromaniac commented Jan 16, 2018

Will probably take a while until I have time to look at this in detail.

Take your time. I may have to make more changes soon anyway, depending on the outcome of #338. Besides, I'm not 100% convinced about the changes to the merge function. I want to try a few more things.

Any reason why we should keep singlebackup? I would prefer multibackup as the default, maybe with an optional -singlebackup.

From the moment I added the -updatebatch switch I tried to keep changes to the default behaviour to a minimum so as to avoid giving someone an ugly surprise. When I wrote the first versions of the script I thought it was safe enough, so multiple backups wouldn't be necessary, but then in practice I found they would be useful.

So, I wouldn't remove single backups because some people may prefer them, but turning them into an optional feature should be fine, I guess.

@claustromaniac claustromaniac changed the title Updater 4.3 Updater for Windows v4.3 Jan 16, 2018
@earthlng
Copy link
Contributor

Looks good and seems to work from what I can tell. But what are these ^M on line 215 + 225?

@claustromaniac
Copy link
Contributor Author

claustromaniac commented Jan 18, 2018

Pretty sure that's Github's syntax validation. Both lines contain double quotes but don't end in double quotes, it may have to do with that.

Edit: apparently it's the CR. Not sure why Github is showing it there, but it shouldn't matter. I assumed Github was trying to validate the syntax because I've seen it do that when I missed ending quotes in the past, and adding the missing quotes effectively removed that ^M.

@earthlng earthlng merged commit 038201f into arkenfox:master Jan 18, 2018
@earthlng
Copy link
Contributor

merged. thanks

@claustromaniac claustromaniac deleted the claustromaniac-patch-1 branch January 18, 2018 16:26
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