Proposal for new names in BLI_path_util
#74506
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#74506
Loading…
Reference in New Issue
No description provided.
Delete Branch "%!s(<nil>)"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
This is my proposal for new names in
BLI_path_util.h
andpath_util.c
. The original names are listed first, the suggested names second.This is not finished (I'll remove this sentence once it is), it's work in progress. However, feel free to comment on what is described here already.
Changed status from 'Needs Triage' to: 'Confirmed'
Added subscriber: @dr.sybren
Added subscriber: @LazyDodo
void BLI_path_basename(const char *path, char *r_filename, const size_t filelen);
parameter name is
r_filename
, so the size field ought to be calledconst size_t filenamelen
same applies to some of the other functions.
Added subscriber: @ideasman42
Note, some terminology we could settle on before going into details, following Python conventions and some conventions already in Blender's code.
Term for the directory component of a path
dirname
.Term for the file without any leading path
basename
.Term for a directory - for functions which only operate on directories
dir
.Term for a
directoryfile - for functions which only operate on namesfilename
.Term of a complete path
filepath
.BLI_setenv
: We could addBLI_os_
for OS wrappers (BLI_setenv
->BLI_os_setenv
), some functions fromstorage.c
could eventually be moved there tooBLI_wstat
for eg.BLI_path_join_and_resolve
+1BLI_path_ensure_directory_exists
:BLI_path_split
+1BLI_path_dirname
/BLI_path_basename
: ... there is a complication.Further Proposals for Renaming
👍
👍
I usually try to distinguish between "the directory name" and "the directory". This is mostly relevant for files, though, where I would consider something like a
FILE *
as "file", andchar *
as "filename".I'm guessing this is a copy-paste error and it's actually a term for a filename, not a directory.
👍
I feel the same way, to me it's an indication that the function is doing too much at once. I'd be happy with two functions, one for resolving
//
-relative paths, and one for joining dirname and filename.Although it makes total sense to me from a naming standpoint, this function is used a lot when trying to write a file. It's quite convenient to have a single function "make sure I can write this file".
This is why I try to make a distinction between "directory" and "directory name": a directory name always exists.
BLI_path_ensure_dir_exists
is fine for me, or maybeBLI_path_mkparent
? At least it reads a bit likemkdir
and it's clear it's about the parent.This is a complication indeed. If there are two functions that do the same, and only differ in their memory management (overwrite existing mem vs. allocate new mem) I wouldn't mind if they were named the same except for some small difference that indicates which one of the two it is. Maybe
BLI_path_basename
andBLI_path_basename_alloc
? I know this is troublesome for patches, as the new name of the non-allocating function would be the same as the old name of the allocating one, potentially creating nasty bugs. However, I'd rather think of what naming scheme we actually want to use, and then think of how to deal with the renaming.All 👍 for me.
I think the alternatives here are clearer. To me they are, anyway.
This task is a bit big, I'd like to split off partially related things where possible:
d14e768069
.It makes sense in general, however we have
BLI_path_
as a prefix, to show that it's operating on paths, not the file/directory arguments.Yes, corrected.
Submitted #75516 (Day of Clean Code: Split wrappers for C API functions into own files).
In this case I think we could leave the name as-is and remove it's use.
Or at least not make it part of this renaming task.
BLI_path_mkparent
seems fine, slight preference forBLI_path_parent_ensure
since it's more in keeping with other names.Neither allocate, one just copies into an existing buffer, while we could do something like what you suggest, call it
BLI_path_basename_copy
, this wouldn't be in keeping with many other functions in this API that copy their output.So I'd still prefer
BLI_path_split_*
functions as suggested.Done (second alternatives).