Fossil Forum

Regularize cmd/page function naming
Login

Regularize cmd/page function naming

(1) By anonymous on 2020-03-04 02:10:02 [source]

I was browsing the Fossil source code to figure out which function corresponds to which Fossil command or UI page. Mostly it's nicely labeled with COMMAND/WEBPAGE tag in preceding comments. I guess, that's what the build process is parsing to bind commands to functions.

But I noticed two intermixed conventions for the underlying function names:

  1. <name>_cmd() for commands, name_page() for UI pages

    egrep "void [^_]*_(cmd|page)" src/*.c
    
  2. cmd_<name>() for commands, page_name() for UI pages

    egrep "void (cmd|page)_" src/*.c
    

There're more entries in the first group, I'd guess it's an original intent, though I'm not sure if that's the case.

Maybe we should regularize the naming for these top-level functions, so the names follow the same convention? It's sort of Fossil API.

Is any of the conventions above more convenient/descriptive, that is preferrable?

(2) By anonymous on 2020-04-06 00:09:58 [link] [source] in reply to 1

Matched the COMMAND and WEBPAGE names to respective _cmd and _page functions [06afb7022], branch: api-cleanup.
Changes are to the function names, not the the commands.

This follows the _cmd/page suffix convention for the API function naming, with underscores used instead of punctuation marks allowed in COMMAND and WEBPAGE names.

Now for the most part the names match; however, a few are left differing (see the list below). I'm not sure if we should match those too for the purposes of making the API convention somewhat uniform.

-------------------

COMMAND                      |  FUNCTION_cmd
-----------------------------|------------------------------
test-compress-2		     |	test_compress2
test-move-repository	     |	test_move_repo
new			     |	create_repository
git			     |	gitmirror
server			     |	webserver
3-way-merge		     |	delta_3waymerge
rebuild			     |	rebuild_database
rss			     |	timeline_rss
smtpd			     |	smtp_server
winsrv			     |	win32_service


WEBPAGE                      |  FUNCTION_page
-----------------------------|------------------------------
favicon.ico		     |	favicon
forume1			     |	forum_main
raw			     |	rawartifact
secureraw		     |	secure_rawartifact
juvlist			     |	uvlist_json

-------------------

(3) By Florian Balmer (florian.balmer) on 2020-04-06 07:03:44 [link] [source] in reply to 2

I don't see the benefit, here. Such large-scale cosmetic changes are hard to test (i.e. did you try every single command?) and make diffs between releases much more complicated than they need to be (i.e. "diff-pollution" absorbing attention away from more critical parts).

(4) By anonymous on 2020-04-06 17:25:04 [link] [source] in reply to 3

...Such large-scale cosmetic changes are hard to test (i.e. did you try every single command?)

The changes are mostly concerned with the top-level API functions. These functions are called from main() by pointer, the bulk of the Fossil code does not care about these names by design.

So the impact of these changes is not a large-scale one, and indeed it's not hard to test. Here's what I did for tests:

  1. 'make' -- builds without errors
  2. 'tclsh ./test/tester.tcl ./fossil -prot' -- Fossil's test suite reports the same test statuses as for the base trunk revision
  3. '0:0' - eye-balled over every line of the changes made, especially those that are not in the function's signature or comments:

    fossil diff -w --from 5e28febf --to api-cleanup | grep "^[-+][^*v][^-+]"
    
  4. Exercised the commands/pages that have the actual code changes (above)

This is a simple refactoring and is a great chance to benefit from the use of the expansive Fossil test suite.

As for the benefit here: the "traditional" naming convention is reasonable; apart from readability, it helps spot instances of one command invoking another. Naming consistency helps new developers navigate the internals of the Fossil code or an occasional back-trace.