gno icon indicating copy to clipboard operation
gno copied to clipboard

feat: add p/moul/helplink

Open moul opened this issue 1 year ago • 3 comments

This PR aimed to promote the use of a p/ library for managing special help links from contracts.

It also provided an opportunity for me to realize that our discussion about changing the $ symbol would require some parsing and detection from the gnoweb perspective. If we want a simple library like this one, the goal should be to ideally craft a link to the current package without specifying the realm path. Relative URLs worked well with ?, but they won't function with $.

As an alternative, we can have this package look for std.PrevRealm().PkgAddr if it is not specified.

cc @jeronimoalbi @thehowl @leohhhn

Related with #2602 Related with #2876

moul avatar Oct 02 '24 17:10 moul

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 63.62%. Comparing base (603f6d3) to head (4824aa9). Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2887      +/-   ##
==========================================
+ Coverage   63.38%   63.62%   +0.24%     
==========================================
  Files         566      566              
  Lines       79490    79656     +166     
==========================================
+ Hits        50388    50685     +297     
+ Misses      25710    25577     -133     
- Partials     3392     3394       +2     
Flag Coverage Δ
contribs/gnodev 60.57% <ø> (ø)
contribs/gnofaucet 14.82% <ø> (ø)
gno.land 67.37% <ø> (ø)
gnovm 67.87% <ø> (ø)
misc/genstd 79.72% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Oct 02 '24 17:10 codecov[bot]

For me, it's ease of use with this kind of library. I found it simpler to just write out the constant up top, put in placeholders for specific arguments, and just populate it with a Sprintf. Helplink still makes me do manual concatenation in some cases, and in situations where there is more than one argument Sprintf's % operator is more practical, imho.

These two code paragraphs have the same output:

	// using helplink
	out += "#### "
	out += hl.Func(strconv.Itoa(i.upvote.Size())+like, "Upvote", "pkgpath", i.pkgpath)
	out += " - "
	out += hl.Func(strconv.Itoa(i.downvote.Size())+like, "Downvote", "pkgpath", i.pkgpath)
	out += "\n\n"

	// using sprintf
	const likesBar = "#### [%d 👍](/r/demo/hof?help&__func=Upvote&pkgpath=%s) - [%d 👎](/r/demo/hof?help&__func=Downvote&pkgPath=%s)\n\n"
	out += ufmt.Sprintf(
		likesBar,
		i.upvote.Size(),
		i.pkgpath,
		i.downvote.Size(),
		i.pkgpath,
	)

The long package-level constant might be a little ugly, but I think Sprintf's practicality beats helplink.

I have to play around a bit more but for now I'm not fully convinced currently.

leohhhn avatar Oct 19 '24 00:10 leohhhn

i think it’s because you’re using a dynamic text with number; maybe in your case you should use the FuncURL and not the Func helper.

moul avatar Oct 19 '24 00:10 moul

TODO: rename helplink to txlink, and make it just about the URL of funcs.

Edit: kept the two packages, with distinct roles.

moul avatar Oct 24 '24 20:10 moul

Based on #2809, I decided to create these two new libraries in my personal realm, p/moul, instead of the usual p/demo. I also updated r/demo/boards to use txlink instead of creating links manually.

There is a question about whether we want to use personal namespaces starting today or if it still makes sense to find a better location.

moul avatar Oct 25 '24 13:10 moul