fix: resolve {build_dir} in translated pages - #2198
Conversation
Resolve translated includes from the language directory when they use
{build_dir}. Keep relative includes based on their source directory.
Fixes git#2197
Signed-off-by: Yang Zhengguo <yangzhgg@gmail.com>
yangzhg
changed the title
|
@dscho Could you please review this PR? Thanks! |
| new_content = categories[line] | ||
| content.gsub!(/include::({build_dir}\/)?(\S+)\.#{ext}/) do | ||
| match = Regexp.last_match | ||
| target = "#{match[2]}.#{ext}" |
There was a problem hiding this comment.
Why not extend the regular expression's second group so that target = match[2] is correct?
| content.gsub!(/include::({build_dir}\/)?(\S+)\.#{ext}/) do | ||
| match = Regexp.last_match | ||
| target = "#{match[2]}.#{ext}" | ||
| base = match[1] ? path.split("/", 2).first : File.dirname(path) |
There was a problem hiding this comment.
How certain can we be that path.split("/", 2) is correct? Are there no subdirectories possible? I am thinking of /path/to/build-dir/Documentation/technical/*, where care needs to be applied to determine the correct replacement for {build_dir}/.
| if categories[line] | ||
| new_content = categories[line] | ||
| content.gsub!(/include::({build_dir}\/)?(\S+)\.#{ext}/) do | ||
| match = Regexp.last_match |
There was a problem hiding this comment.
Couldn't this be done more elegantly via |_, build_dir, path|? (I don't know, I'm no longer fluent in Ruby.)
| get_content_f = proc do |source, target| | ||
| name = File.join(File.dirname(source), target) | ||
| get_content_f = proc do |base, target| |
There was a problem hiding this comment.
The rename makes it unnecessarily hard to spot the actual functional change: removing the File.dirname(). And without an adequate explanation in the commit message, the cognitive load required to review this change is sub-optimally big. Please fix that.
2 participants
The translated manual page updater leaves
{build_dir}in include paths, so the command lists are replaced by missing-file warnings.Resolve those includes from the language directory. Regular relative includes still use the directory of the source file.
I checked the current
git-html-l10ndata fores,fr,pt_BR,ru,sv,uk, andzh_HANS-CN, including nestedconfig/merge.adocfiles where available. The legacy.txtpath still works as well.Fixes #2197