Add CLI command 'preview' and API 'aug_preview' to preview file contents #690
Conversation
|
That looks very interesting! At the moment, this feature is achieved in various tools by leveraging the |
|
The preview function has to read the original file (again) in order to apply the transform to it, so it has to do a 2nd read of the original file. It does save having to write the file out and having to read the resulting file back off the disk (by whatever is wants to do the diff), so it saves IO operations. |
src/augeas.c
Outdated
|
|
||
| ERR_THROW(file_path == NULL, aug, AUG_EBADARG, "Path %s is not associated with a file", path); | ||
|
|
||
| if ( file_path == NULL ) { |
There was a problem hiding this comment.
This if is unnecessary since you know from the ERR_THROW that file_path != NULL
src/augeas.c
Outdated
| } | ||
|
|
||
| r = xasprintf(&lens_path, "%s%s/%s", AUGEAS_META_TREE, file_path, s_lens); | ||
| if ( r>= 0 ) { |
There was a problem hiding this comment.
This is super nitpicky, but I generally write if statements with now spaces after the parentheses, but spaces around the operator, i.e., if (r >= 0) { .. - I wish there was some easy-to-use code formatter that we could just run to make sure these things are consistent. In any event, I would prefer to have these things unfirom as much as possible, but it's a preference, not a "I would never merge a PR formatted like this" ;)
There was a problem hiding this comment.
Instead of the if statement, it would be better to write
ERR_NOMEM(lens_path == NULL, aug)
There was a problem hiding this comment.
Replaced if (r >= 0) with ERR_NOMEM macro
src/augeas.c
Outdated
|
|
||
| r = xasprintf(&lens_path, "%s%s/%s", AUGEAS_META_TREE, file_path, s_lens); | ||
| if ( r>= 0 ) { | ||
| r = aug_get(aug,lens_path,(const char **) &lens_name); |
There was a problem hiding this comment.
You don't need the assignment to r in what follows - better to leave that out so the reader doesn't wonder why it's assigned to.
There was a problem hiding this comment.
Removed unused assignment
src/augeas.c
Outdated
|
|
||
| ERR_THROW(lens_name == NULL, aug, AUG_ENOLENS, "No lens found for path %s", path); | ||
|
|
||
| if ( STREQLEN(file_path, AUGEAS_FILES_TREE, strlen(AUGEAS_FILES_TREE) ) ) { |
There was a problem hiding this comment.
This if does not seem necessary - we already constructed file_path to have the 'right' content, so we should be able to perform the body of the if unconditionally.
There was a problem hiding this comment.
Removed if ( STREQLEN
src/augeas.c
Outdated
| ERR_NOMEM(source_filename == NULL, aug); | ||
| source_text = xread_file(source_filename); | ||
| } | ||
| if ( source_text == NULL ) { |
There was a problem hiding this comment.
I think a NULL here should be reported as an error to the caller - they told us to read a file and we couldn't.
src/augeas.c
Outdated
| free(lens_path); | ||
| free(source_filename); | ||
| free(source_text); | ||
| api_exit(aug); |
There was a problem hiding this comment.
Since it's easy to only fix up all the free calls in the good path or the error path when this code changes in the future, I usually write functions like this as
int something(..) {
int result = -1;
.. do things ..
/* everything worked out */
result = 0;
error:
/* free all the things */
return result;
}
There was a problem hiding this comment.
I think that's a better structure. Updated.
tests/test-api.c
Outdated
| if ( hosts_fp ) { | ||
| hosts_txt = calloc(sizeof(char),4096); | ||
| if ( hosts_txt ) { | ||
| readsz = fread(hosts_txt,sizeof(char),4096,hosts_fp); |
There was a problem hiding this comment.
Looks like there's some errant whitespace
This Pull Requests adds a new command
previewto augtool, and a new functionaug_preview()to the API.The
previewcommand accepts a path-name as an argument.For the file corresponding to the path-name, the contents of the file are written to stdout, with the current
nodes and values from the
/filestree applied.This corresponds to the contents of the file that would be written by a subsequent
saveoperation.The path-expression must evaluate to a single node only, and the source-file is determined from this node.
The API call
aug_preview()places the resulting file-contents intochar *OUT, allocating the required memory.