full SHA is the default - but prints out the subset

[muschellij2]

This will fail the checks because the default behavior changes. There is no full_sha argument, but I simply changed the printing to first 7 characters, but the object itself still has the full SHA.

[gaborcsardi]

Thanks! Do you mind if I simplify the regex + replacement part? It is hard for me to read. (If I can't simplify it, then I'll just leave it as is.)

[muschellij2]

Agreed. Here's a quick and dirty explanation

src <- x$source
# look for an @ symbol
# need the / because git@github.com:blah/blah@asdfasdf
  sha_regex <- ".*/(.*@.*)\\)"
  has_sha <- grepl(sha_regex, src)
  src <- src[has_sha]
# src is only those with an @ symbol
  if (any(has_sha)) {
  # Take everything past the /, including SHA
    sub_src <- sub(sha_regex, "\\1", src)
# remove the front (aka the package) or repo/package or repo/package.git
    sha <- sub(".*@", "", sub_src)
    sha <- substr(sha, 1, 7)
# now put the SHA after the @ - need mapply because not vectorized sub
    sha <- mapply(function(orig, replacement) {
      sub("@(.*)", paste0("@", replacement), x = orig)
    }, sub_src, sha)
# vectorized (1-to-1 mapping) and now put the "front" with back (e.g. front is "Github (git@github.com:user" and 
# back is repo@SHA)
    src <- mapply(function(orig, replacement) {
      sub("(.*)/(.*@.*)\\)", paste0("\\1", "/", replacement, ")"), x = orig)
    }, src, sha)
    src <- unname(src)
    x$source[has_sha] <- src
  }

[codecov-commenter]

Codecov Report

Merging #61 (249d4fc) into main (473bbf2) will increase coverage by 0.16%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #61      +/-   ##
==========================================
+ Coverage   84.08%   84.24%   +0.16%     
==========================================
  Files          14       14              
  Lines         710      711       +1     
==========================================
+ Hits          597      599       +2     
+ Misses        113      112       -1     

Impacted Files	Coverage Δ
R/package-info.R	88.02% <100.00%> (+0.67%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 473bbf2...249d4fc. Read the comment docs.

[gaborcsardi]

Thanks for the explanation! Maybe I am missing something, but I think we can use this simple substitution, no?

  sub("([0-9a-f]{7})[0-9a-f]{33}", "\\1", x)

https://github.com/r-lib/sessioninfo/pull/61/files#diff-1bf578267d76f849c4d53b6ee6642a93ae375a84ec39b0e6421f05908c894b08R223-R225

I don't think a 40 character long hexa string can appear anywhere else, realistically.

[muschellij2]

That’s totally fine. I am less of a regex wizard.

On Sat, Dec 4, 2021 at 5:08 PM Gábor Csárdi ***@***.***> wrote: Thanks for the explanation! Maybe I am missing something, but I think we can use this simple substitution, no? sub("([0-9a-f]{7})[0-9a-f]{33}", "\\1", x) https://github.com/r-lib/sessioninfo/pull/61/files#diff-1bf578267d76f849c4d53b6ee6642a93ae375a84ec39b0e6421f05908c894b08R223-R225 I don't think a 40 character long hexa string can appear anywhere else, realistically. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#61 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIGPLVFC43444VLVGGGK53UPKGOZANCNFSM5JJY6O7Q> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

-- Best, John

[gaborcsardi]

Thanks. Your approach is probably more robust, but I think in this restricted setting my crude approach is fine, and it'll be easier to maintain for me.

Thanks again, I'll merge once the the CI is done!