sheets-url-shortener icon indicating copy to clipboard operation
sheets-url-shortener copied to clipboard

Write additional metadata back into the source sheet

Open micrictor opened this issue 2 years ago • 4 comments

There's three changes in this PR - let me know if you would prefer I break it up into three separate PRs.

  1. Make redirect matching case-insensitive. Since the map keys were already normalized to lowercase, this fixes an issue where mixed case redirects wouldn't work.
  2. Upon redirect, increment the third column ('Hit Count') and write the current time to a fourth column ('Last Visited') for the row of the redirect.
  3. Permit users to specify their desired redirect code via environment variable. Primarily, I'd expect users to use this to change between the default permanent redirect and a temporary redirect.

micrictor avatar Jan 22 '23 16:01 micrictor

I don't think I'll accept the write requests back into the sheet. They will be buggy on concurrent instances. Resp codes do not sound very useful either. Feel free to keep these in your fork.

ahmetb avatar Jan 22 '23 17:01 ahmetb

Fair enough. Do you want the case-insensitive bit? I can cherry pick it out if you do, otherwise no worries - and thank you for making this!

On Sun, Jan 22, 2023 at 9:15 AM Ahmet Alp Balkan @.***> wrote:

I don't think I'll accept the write requests back into the sheet. They will be buggy on concurrent instances. Resp codes do not sound very useful either.

— Reply to this email directly, view it on GitHub https://github.com/ahmetb/sheets-url-shortener/pull/4#issuecomment-1399550291, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABTCAGN4Y5NTIDKJTRFQYZLWTVTMHANCNFSM6AAAAAAUDCO7PE . You are receiving this because you authored the thread.Message ID: @.***>

micrictor avatar Jan 22 '23 17:01 micrictor

Even then, I don't wanna break anyone's workflow without them noticing. I personally don't use uppercase at all and it does not seem like I'd need it.

ahmetb avatar Jan 22 '23 20:01 ahmetb

I don't think it would be a breaking change for lookups that currently work - the lookup key is already lowercased here:

https://github.com/ahmetb/sheets-url-shortener/blob/master/main.go#L189-L200

I believe this means is that if I have a shortcut ShortCut, it is impossible for it to match a URL path.

It may possibly impact users if they depend on mixed-case shortcuts not ever redirecting - e.g. if you have a shortcut for gh and need http://example.local/GH to not redirect.

micrictor avatar Jan 22 '23 21:01 micrictor