Add diff support for renamed/copied file changes on commit page #7132

Closed
opened 2025-11-02 07:16:30 -06:00 by GiteaMirror · 16 comments
Owner

Originally created by @joseluisq on GitHub (Apr 7, 2021).

  • Gitea version (or commit ref): 1.13.7
  • Git version: 2.31.1
  • Operating system: Linux 5.11.11-arch1-1 x86_64
  • Database (use [x]):
    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL)
    • No
  • Log gist:

Description

I have realized that Gitea is not supporting diff changes for renamed/copied files on the commit page view.
Page URL: /username/repo/commit/hash

Screenshots

For instance, I have a commit which contains a file renamed but with some additions and deletions.
Below an extract of my git show

src/core/{render.ts => dom.ts} | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------
# ....
diff --git a/src/core/render.ts b/src/core/dom.ts
similarity index 78%
rename from src/core/render.ts
rename to src/core/dom.ts
index bc123cb..1230ccd 100644
--- a/src/core/render.ts
+++ b/src/core/dom.ts
@@ -3,28 +3,63 @@

My full Git diff patch file content:

0001-feat-tagged-html-templates-support.patch
From e3b3525a8568de55cd33c068755ff232af86ab25 Mon Sep 17 00:00:00 2001
From: Jose Quintana <joseluisquintana20@gmail.com>
Date: Wed, 7 Apr 2021 23:13:52 +0200
Subject: [PATCH] feat: tagged html templates support

Signed-off-by: Jose Quintana <joseluisquintana20@gmail.com>
---
 package.json                   |  2 +-
 src/components/button.ts       |  7 ++++
 src/core/component.ts          |  2 +-
 src/core/{render.ts => dom.ts} | 74 +++++++++++++++++++++++++---------
 src/core/index.ts              |  2 +-
 src/index.ts                   | 17 ++++----
 yarn.lock                      |  8 ++--
 7 files changed, 77 insertions(+), 35 deletions(-)
 rename src/core/{render.ts => dom.ts} (78%)

diff --git a/package.json b/package.json
index f97cb36..ff225bc 100644
--- a/package.json
+++ b/package.json
@@ -8,7 +8,7 @@
         "lint": "make lint"
     },
     "devDependencies": {
-        "esbuild": "^0.11.5",
+        "esbuild": "^0.11.6",
         "tslint": "^6.1.3",
         "tslint-config-standard-plus": "^2.3.0",
         "typescript": "^4.2.3"
diff --git a/src/components/button.ts b/src/components/button.ts
index b08fc26..e607aa8 100644
--- a/src/components/button.ts
+++ b/src/components/button.ts
@@ -11,9 +11,16 @@ export class Button implements Component {
         return html`
             <div>
                 <button @click="onClick">Child button click!</button>
+
+                <!-- 1. -->
                 <ul>
                     <li @for="(v, _) in alpha">{v}</li>
                 </ul>
+
+                <!-- 2. -->
+                <ol>
+                   ${this.alpha.map((v) => html`<li>${v}</li>`)}
+                </ol>
             </div>
         `
     }
diff --git a/src/core/component.ts b/src/core/component.ts
index 4c4cb02..b8e54c6 100644
--- a/src/core/component.ts
+++ b/src/core/component.ts
@@ -1,3 +1,3 @@
 export interface Component {
-    render (): HTMLElement | null
+    render (): string
 }
diff --git a/src/core/render.ts b/src/core/dom.ts
similarity index 78%
rename from src/core/render.ts
rename to src/core/dom.ts
index cb633bb..2781cfd 100644
--- a/src/core/render.ts
+++ b/src/core/dom.ts
@@ -3,28 +3,63 @@ import { Component } from "./component"
 const PLACEHOLDER = /{\s?([a-zA-Z_]+([0-9a-zA-Z_]+)?)\s?}/
 const DIR_FOR = /^\(([a-zA-Z_]+([0-9a-zA-Z_]+)?)(\s?,\s?)([a-zA-Z_]+([0-9a-zA-Z_]+)?)?\)\ in\ ([a-zA-Z_]+([0-9a-zA-Z_]+)?)$/
 
-/** It converts a HTML string literal into a DOM template element. */
-export function html (...html: any[]) {
-    if (!Array.isArray(html) || html.length === 0) {
+/** It converts a tagged HTML template into a valid HTML template string. */
+export function html (...literals: any[]) {
+    if (!Array.isArray(literals) || literals.length === 0) {
+        return ""
+    }
+
+    const rootArr = literals[0] as string[]
+
+    // TODO: Validate some data types
+    let str = ""
+    for (let i = 0; i < literals.length; i++) {
+        if (i === 0) {
+            str += rootArr[i]
+        } else {
+            const h = literals[i]
+            if (Array.isArray(h)) {
+                str += h.join("") + rootArr[i]
+            } else {
+                str += literals[i] + rootArr[i]
+            }
+        }
+    }
+
+    return str.trim()
+}
+
+/** It creates a document fragment for a given HTML markup string. */
+function createFragment (htmlStr: string) {
+    htmlStr = htmlStr.trim()
+
+    if (htmlStr === "") {
         return null
     }
-    console.log(html)
-    const str = html.join("").trim()
 
     const tmpl = document.createElement("template")
-    tmpl.innerHTML = str
+    tmpl.innerHTML = htmlStr
+
+    if (tmpl.content.childNodes.length === 0) {
+        throw new Error("HTML element has no root element.")
+    }
 
-    // TODO: validate to require only one root element.
-    const node = tmpl.content.firstChild
+    if (tmpl.content.childNodes.length === 1) {
+        return tmpl.content
+    }
+
+    if (tmpl.content.childNodes.length > 1) {
+        throw new Error("HTML element has many root elements. Only one is required.")
+    }
 
-    return node as HTMLElement | null
+    return null
 }
 
 /**
  * It handles an iterator which walks the DOM tree of the root component from top to bottom
- * filtering only element types.
+ * filtering only element types and returning a root fragment afterwards.
  */
-function renderComponentAsElement (baseComp: Component) {
+function componentAsFragment (baseComp: Component) {
     // tslint:disable-next-line
     if (typeof baseComp.render === "undefined") {
         throw new Error(
@@ -32,14 +67,14 @@ function renderComponentAsElement (baseComp: Component) {
         )
     }
 
-    const baseNode = baseComp.render()
+    const baseNode = createFragment(baseComp.render())
 
     if (!baseNode) return null
 
     let currentNode: HTMLElement | null
     const iterator = document.createNodeIterator(baseNode, NodeFilter.SHOW_ELEMENT)
 
-    const parentNodes: [Component | null, HTMLElement][] = [ [ baseComp, baseNode ] ]
+    const parentNodes: [Component | null, HTMLElement | DocumentFragment][] = [ [ baseComp, baseNode ] ]
     let counter = 0
 
     // TODO: Add proper error handling
@@ -61,9 +96,9 @@ function renderComponentAsElement (baseComp: Component) {
                         if (!bindComp) {
                             currentNode.remove()
                         } else {
-                            const bindNode = bindComp.render()
+                            const bindNode = createFragment(bindComp.render())
                             if (bindNode) {
-                                counter = parentNodes.push([ bindComp , currentNode ]) - 1
+                                counter = parentNodes.push([ bindComp, currentNode ]) - 1
                                 currentNode.appendChild(bindNode)
                                 currentNode.setAttribute("a:idx", counter.toString())
                             }
@@ -98,6 +133,7 @@ function renderComponentAsElement (baseComp: Component) {
                 // TODO: Add support for more events
 
                 // 2. Loops
+                // TODO: @for is possibly not needed since tagged templates is now supported
                 const loop = currentNode.getAttribute("@for") || ""
                 if (loop) {
                     const parts = loop.match(DIR_FOR)
@@ -172,12 +208,12 @@ function renderComponentAsElement (baseComp: Component) {
     return baseNode
 }
 
-/** It renders a component and then append it to a root element. */
-export function Render (component: Component, root: HTMLElement = document.body) {
-    const el = renderComponentAsElement(component)
+/** It renders a component and then append it to a given element. */
+export function render (component: Component, target: HTMLElement) {
+    const el = componentAsFragment(component)
 
     if (el) {
-        root.appendChild(el)
+        target.appendChild(el)
     } else {
         throw new Error(
             "A root HTML element is required for `"
diff --git a/src/core/index.ts b/src/core/index.ts
index f708bde..e3f08a3 100644
--- a/src/core/index.ts
+++ b/src/core/index.ts
@@ -1,2 +1,2 @@
+export * from "./dom"
 export * from "./component"
-export * from "./render"
diff --git a/src/index.ts b/src/index.ts
index a4b4326..5c49018 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -1,9 +1,9 @@
-import { Component, html, Render } from "./core"
+import { Component, html, render } from "./core"
 import { Button } from "./components"
 
 class App implements Component {
-    public bits = [ 16, 32, 64, 128, 256 ]
     public $btn: Button
+    public bits = [ { v: 16 }, { v: 32 }, { v: 64 }, { v: 128 }, { v: 256 } ]
 
     constructor () {
         this.$btn = new Button()
@@ -16,14 +16,12 @@ class App implements Component {
 
     render () {
         return html`
-            <main>
+            <div>
                 <h1>Component</h1>
-                <a href="#" @click="onClick">Click on link!</a> <br>
+                <a href="#" @click="onClick">Link!</a> <br>
                 <include @id="btn"></include>
-                <ul>
-                    <li @for="(v, _) in bits">{v}</li>
-                </ul>
-            </main>
+                <ul>${this.bits.map((item) => html`<li>${item.v}</li>`)}</ul>
+            </div>
         `
     }
 }
@@ -31,5 +29,6 @@ class App implements Component {
 window.addEventListener("load", () => {
     const root = document.getElementById("root") || document.createElement("div")
     root.setAttribute("id", "root")
-    Render(new App(), root)
+
+    render(new App(), root)
 })
diff --git a/yarn.lock b/yarn.lock
index f554130..a47ea17 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -99,10 +99,10 @@ doctrine@0.7.2:
     esutils "^1.1.6"
     isarray "0.0.1"
 
-esbuild@^0.11.5:
-  version "0.11.5"
-  resolved "https://registry.yarnpkg.com/esbuild/-/esbuild-0.11.5.tgz#25b18a2ff2fb9580683edce26a48f64c08c2f2df"
-  integrity sha512-aRs6jAE+bVRp1tyfzUugAw1T/Y0Fwzp4Z2ROikF3h+UifoD5QlEbEYQGc6orNnnSIRhWR5VWBH7LozlAumaLHg==
+esbuild@^0.11.6:
+  version "0.11.6"
+  resolved "https://registry.yarnpkg.com/esbuild/-/esbuild-0.11.6.tgz#20961309c4cfed00b71027e18806150358d0cbb0"
+  integrity sha512-L+nKW9ftVS/N2CVJMR9YmXHbkm+vHzlNYuo09rzipQhF7dYNvRLfWoEPSDRTl10and4owFBV9rJ2CTFNtLIOiw==
 
 escape-string-regexp@^1.0.5:
   version "1.0.5"
-- 
2.31.1

However what I get in the GUI is this empty section:

image

So I would be great if Gitea could add support for this.

Originally created by @joseluisq on GitHub (Apr 7, 2021). <!-- NOTE: If your issue is a security concern, please send an email to security@gitea.io instead of opening a public issue --> <!-- 1. Please speak English, this is the language all maintainers can speak and write. 2. Please ask questions or configuration/deploy problems on our Discord server (https://discord.gg/gitea) or forum (https://discourse.gitea.io). 3. Please take a moment to check that your issue doesn't already exist. 4. Make sure it's not mentioned in the FAQ (https://docs.gitea.io/en-us/faq) 5. Please give all relevant information below for bug reports, because incomplete details will be handled as an invalid report. --> - Gitea version (or commit ref): 1.13.7 - Git version: 2.31.1 - Operating system: Linux 5.11.11-arch1-1 x86_64 <!-- Please include information on whether you built gitea yourself, used one of our downloads or are using some other package --> <!-- Please also tell us how you are running gitea, e.g. if it is being run from docker, a command-line, systemd etc. ---> <!-- If you are using a package or systemd tell us what distribution you are using --> - Database (use `[x]`): - [x] PostgreSQL - [ ] MySQL - [ ] MSSQL - [ ] SQLite - Can you reproduce the bug at https://try.gitea.io: - [ ] Yes (provide example URL) - [ ] No - Log gist: <!-- It really is important to provide pertinent logs --> <!-- Please read https://docs.gitea.io/en-us/logging-configuration/#debugging-problems --> <!-- In addition, if your problem relates to git commands set `RUN_MODE=dev` at the top of app.ini --> ## Description <!-- If using a proxy or a CDN (e.g. CloudFlare) in front of gitea, please disable the proxy/CDN fully and connect to gitea directly to confirm the issue still persists without those services. --> I have realized that Gitea is not supporting diff changes for renamed/copied files on the commit page view. Page URL: `/username/repo/commit/hash` ## Screenshots <!-- **If this issue involves the Web Interface, please include a screenshot** --> For instance, I have a commit which contains a file renamed but with some additions and deletions. Below an extract of my `git show` ```diff src/core/{render.ts => dom.ts} | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------- # .... diff --git a/src/core/render.ts b/src/core/dom.ts similarity index 78% rename from src/core/render.ts rename to src/core/dom.ts index bc123cb..1230ccd 100644 --- a/src/core/render.ts +++ b/src/core/dom.ts @@ -3,28 +3,63 @@ ``` My full Git diff patch file content: <details><summary><b>0001-feat-tagged-html-templates-support.patch</b></summary> ```diff From e3b3525a8568de55cd33c068755ff232af86ab25 Mon Sep 17 00:00:00 2001 From: Jose Quintana <joseluisquintana20@gmail.com> Date: Wed, 7 Apr 2021 23:13:52 +0200 Subject: [PATCH] feat: tagged html templates support Signed-off-by: Jose Quintana <joseluisquintana20@gmail.com> --- package.json | 2 +- src/components/button.ts | 7 ++++ src/core/component.ts | 2 +- src/core/{render.ts => dom.ts} | 74 +++++++++++++++++++++++++--------- src/core/index.ts | 2 +- src/index.ts | 17 ++++---- yarn.lock | 8 ++-- 7 files changed, 77 insertions(+), 35 deletions(-) rename src/core/{render.ts => dom.ts} (78%) diff --git a/package.json b/package.json index f97cb36..ff225bc 100644 --- a/package.json +++ b/package.json @@ -8,7 +8,7 @@ "lint": "make lint" }, "devDependencies": { - "esbuild": "^0.11.5", + "esbuild": "^0.11.6", "tslint": "^6.1.3", "tslint-config-standard-plus": "^2.3.0", "typescript": "^4.2.3" diff --git a/src/components/button.ts b/src/components/button.ts index b08fc26..e607aa8 100644 --- a/src/components/button.ts +++ b/src/components/button.ts @@ -11,9 +11,16 @@ export class Button implements Component { return html` <div> <button @click="onClick">Child button click!</button> + + <!-- 1. --> <ul> <li @for="(v, _) in alpha">{v}</li> </ul> + + <!-- 2. --> + <ol> + ${this.alpha.map((v) => html`<li>${v}</li>`)} + </ol> </div> ` } diff --git a/src/core/component.ts b/src/core/component.ts index 4c4cb02..b8e54c6 100644 --- a/src/core/component.ts +++ b/src/core/component.ts @@ -1,3 +1,3 @@ export interface Component { - render (): HTMLElement | null + render (): string } diff --git a/src/core/render.ts b/src/core/dom.ts similarity index 78% rename from src/core/render.ts rename to src/core/dom.ts index cb633bb..2781cfd 100644 --- a/src/core/render.ts +++ b/src/core/dom.ts @@ -3,28 +3,63 @@ import { Component } from "./component" const PLACEHOLDER = /{\s?([a-zA-Z_]+([0-9a-zA-Z_]+)?)\s?}/ const DIR_FOR = /^\(([a-zA-Z_]+([0-9a-zA-Z_]+)?)(\s?,\s?)([a-zA-Z_]+([0-9a-zA-Z_]+)?)?\)\ in\ ([a-zA-Z_]+([0-9a-zA-Z_]+)?)$/ -/** It converts a HTML string literal into a DOM template element. */ -export function html (...html: any[]) { - if (!Array.isArray(html) || html.length === 0) { +/** It converts a tagged HTML template into a valid HTML template string. */ +export function html (...literals: any[]) { + if (!Array.isArray(literals) || literals.length === 0) { + return "" + } + + const rootArr = literals[0] as string[] + + // TODO: Validate some data types + let str = "" + for (let i = 0; i < literals.length; i++) { + if (i === 0) { + str += rootArr[i] + } else { + const h = literals[i] + if (Array.isArray(h)) { + str += h.join("") + rootArr[i] + } else { + str += literals[i] + rootArr[i] + } + } + } + + return str.trim() +} + +/** It creates a document fragment for a given HTML markup string. */ +function createFragment (htmlStr: string) { + htmlStr = htmlStr.trim() + + if (htmlStr === "") { return null } - console.log(html) - const str = html.join("").trim() const tmpl = document.createElement("template") - tmpl.innerHTML = str + tmpl.innerHTML = htmlStr + + if (tmpl.content.childNodes.length === 0) { + throw new Error("HTML element has no root element.") + } - // TODO: validate to require only one root element. - const node = tmpl.content.firstChild + if (tmpl.content.childNodes.length === 1) { + return tmpl.content + } + + if (tmpl.content.childNodes.length > 1) { + throw new Error("HTML element has many root elements. Only one is required.") + } - return node as HTMLElement | null + return null } /** * It handles an iterator which walks the DOM tree of the root component from top to bottom - * filtering only element types. + * filtering only element types and returning a root fragment afterwards. */ -function renderComponentAsElement (baseComp: Component) { +function componentAsFragment (baseComp: Component) { // tslint:disable-next-line if (typeof baseComp.render === "undefined") { throw new Error( @@ -32,14 +67,14 @@ function renderComponentAsElement (baseComp: Component) { ) } - const baseNode = baseComp.render() + const baseNode = createFragment(baseComp.render()) if (!baseNode) return null let currentNode: HTMLElement | null const iterator = document.createNodeIterator(baseNode, NodeFilter.SHOW_ELEMENT) - const parentNodes: [Component | null, HTMLElement][] = [ [ baseComp, baseNode ] ] + const parentNodes: [Component | null, HTMLElement | DocumentFragment][] = [ [ baseComp, baseNode ] ] let counter = 0 // TODO: Add proper error handling @@ -61,9 +96,9 @@ function renderComponentAsElement (baseComp: Component) { if (!bindComp) { currentNode.remove() } else { - const bindNode = bindComp.render() + const bindNode = createFragment(bindComp.render()) if (bindNode) { - counter = parentNodes.push([ bindComp , currentNode ]) - 1 + counter = parentNodes.push([ bindComp, currentNode ]) - 1 currentNode.appendChild(bindNode) currentNode.setAttribute("a:idx", counter.toString()) } @@ -98,6 +133,7 @@ function renderComponentAsElement (baseComp: Component) { // TODO: Add support for more events // 2. Loops + // TODO: @for is possibly not needed since tagged templates is now supported const loop = currentNode.getAttribute("@for") || "" if (loop) { const parts = loop.match(DIR_FOR) @@ -172,12 +208,12 @@ function renderComponentAsElement (baseComp: Component) { return baseNode } -/** It renders a component and then append it to a root element. */ -export function Render (component: Component, root: HTMLElement = document.body) { - const el = renderComponentAsElement(component) +/** It renders a component and then append it to a given element. */ +export function render (component: Component, target: HTMLElement) { + const el = componentAsFragment(component) if (el) { - root.appendChild(el) + target.appendChild(el) } else { throw new Error( "A root HTML element is required for `" diff --git a/src/core/index.ts b/src/core/index.ts index f708bde..e3f08a3 100644 --- a/src/core/index.ts +++ b/src/core/index.ts @@ -1,2 +1,2 @@ +export * from "./dom" export * from "./component" -export * from "./render" diff --git a/src/index.ts b/src/index.ts index a4b4326..5c49018 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,9 +1,9 @@ -import { Component, html, Render } from "./core" +import { Component, html, render } from "./core" import { Button } from "./components" class App implements Component { - public bits = [ 16, 32, 64, 128, 256 ] public $btn: Button + public bits = [ { v: 16 }, { v: 32 }, { v: 64 }, { v: 128 }, { v: 256 } ] constructor () { this.$btn = new Button() @@ -16,14 +16,12 @@ class App implements Component { render () { return html` - <main> + <div> <h1>Component</h1> - <a href="#" @click="onClick">Click on link!</a> <br> + <a href="#" @click="onClick">Link!</a> <br> <include @id="btn"></include> - <ul> - <li @for="(v, _) in bits">{v}</li> - </ul> - </main> + <ul>${this.bits.map((item) => html`<li>${item.v}</li>`)}</ul> + </div> ` } } @@ -31,5 +29,6 @@ class App implements Component { window.addEventListener("load", () => { const root = document.getElementById("root") || document.createElement("div") root.setAttribute("id", "root") - Render(new App(), root) + + render(new App(), root) }) diff --git a/yarn.lock b/yarn.lock index f554130..a47ea17 100644 --- a/yarn.lock +++ b/yarn.lock @@ -99,10 +99,10 @@ doctrine@0.7.2: esutils "^1.1.6" isarray "0.0.1" -esbuild@^0.11.5: - version "0.11.5" - resolved "https://registry.yarnpkg.com/esbuild/-/esbuild-0.11.5.tgz#25b18a2ff2fb9580683edce26a48f64c08c2f2df" - integrity sha512-aRs6jAE+bVRp1tyfzUugAw1T/Y0Fwzp4Z2ROikF3h+UifoD5QlEbEYQGc6orNnnSIRhWR5VWBH7LozlAumaLHg== +esbuild@^0.11.6: + version "0.11.6" + resolved "https://registry.yarnpkg.com/esbuild/-/esbuild-0.11.6.tgz#20961309c4cfed00b71027e18806150358d0cbb0" + integrity sha512-L+nKW9ftVS/N2CVJMR9YmXHbkm+vHzlNYuo09rzipQhF7dYNvRLfWoEPSDRTl10and4owFBV9rJ2CTFNtLIOiw== escape-string-regexp@^1.0.5: version "1.0.5" -- 2.31.1 ``` </details> However what I get in the GUI is this empty section: ![image](https://user-images.githubusercontent.com/1700322/113938661-2fd4e900-97fb-11eb-86bb-55c762e4d9f4.png) So I would be great if Gitea could add support for this.
GiteaMirror added the type/enhancementtype/bug labels 2025-11-02 07:16:30 -06:00
Author
Owner

@lunny commented on GitHub (Apr 8, 2021):

@joseluisq Could you provide the diff patch file?

@lunny commented on GitHub (Apr 8, 2021): @joseluisq Could you provide the diff patch file?
Author
Owner

@zeripath commented on GitHub (Apr 8, 2021):

Yes, please replicate this on try.gitea.io

@zeripath commented on GitHub (Apr 8, 2021): Yes, please replicate this on try.gitea.io
Author
Owner

@zeripath commented on GitHub (Apr 8, 2021):

And tell us what version of git you're using - it's highly relevant

@zeripath commented on GitHub (Apr 8, 2021): And tell us what version of git you're using - it's highly relevant
Author
Owner

@joseluisq commented on GitHub (Apr 8, 2021):

Ok, I will try to replicate this on try.gitea.io.

I have locally git 2.31.1

@joseluisq commented on GitHub (Apr 8, 2021): Ok, I will try to replicate this on try.gitea.io. I have locally git 2.31.1
Author
Owner

@joseluisq commented on GitHub (Apr 8, 2021):

@joseluisq Could you provide the diff patch file?

@lunny full diff patch file content in the issue description (updated).

@joseluisq commented on GitHub (Apr 8, 2021): > @joseluisq Could you provide the diff patch file? @lunny full diff patch file content in the issue description (updated).
Author
Owner

@joseluisq commented on GitHub (Apr 8, 2021):

@zeripath question

try.gitea.io is using 1.15.0+dev-81-g298d56fe8 do I still need to test it there?
Since I'm facing this issue in the stable 1.13.7 release?

@joseluisq commented on GitHub (Apr 8, 2021): @zeripath question `try.gitea.io` is using `1.15.0+dev-81-g298d56fe8` do I still need to test it there? Since I'm facing this issue in the stable 1.13.7 release?
Author
Owner

@zeripath commented on GitHub (Apr 8, 2021):

yes - it would be useful to know if it is present in master - and if you can present a case that would fail on your set-up but does not on try we can try it on various configurations to understand what is making it a heisenbug.

My suspicion is that the case is related to improvements in git's diff output so we wouldn't have seen it or handled it before.

I'll take another look at the patch parsing code to see if the issue is there - but as I say it would be excellent to have a testcase to that we can show where it fails. -- (ah I see you've updated your comment with the patch - that is extremely helpful!)

@zeripath commented on GitHub (Apr 8, 2021): yes - it would be useful to know if it is present in master - and if you can present a case that would fail on your set-up but does not on try we can try it on various configurations to understand what is making it a heisenbug. My suspicion is that the case is related to improvements in git's diff output so we wouldn't have seen it or handled it before. I'll take another look at the patch parsing code to see if the issue is there - but as I say it would be excellent to have a testcase to that we can show where it fails. -- (ah I see you've updated your comment with the patch - that is extremely helpful!)
Author
Owner

@joseluisq commented on GitHub (Apr 8, 2021):

Alright, I will give it a test during the day then.

@joseluisq commented on GitHub (Apr 8, 2021): Alright, I will give it a test during the day then.
Author
Owner

@zeripath commented on GitHub (Apr 8, 2021):

OK on master and origin/release/v1.13 ParsePatch is handling this correctly - so the issue is going to be at the template level I suspect

@zeripath commented on GitHub (Apr 8, 2021): OK on master and origin/release/v1.13 ParsePatch is handling this correctly - so the issue is going to be at the template level I suspect
Author
Owner

@zeripath commented on GitHub (Apr 8, 2021):

yup and looking at templates/repo/diff/box.tmpl:

05b7e32829/templates/repo/diff/box.tmpl (L52-L54)

05b7e32829/templates/repo/diff/box.tmpl (L90-L92)

We can see that the compare box won't display if the file is marked as IsRenamed.

So in which case this is likely a really simple fix - we just need to figure out what happens if we drop that IsRenamed test and if that works fine when there is no diff then boom we're done.


So the above is just dealing with the header bar. We need to fix here to show the diff:

05b7e32829/templates/repo/diff/box.tmpl (L116)

@zeripath commented on GitHub (Apr 8, 2021): yup and looking at templates/repo/diff/box.tmpl: https://github.com/go-gitea/gitea/blob/05b7e328297142c7ddb888339901524708472a3a/templates/repo/diff/box.tmpl#L52-L54 https://github.com/go-gitea/gitea/blob/05b7e328297142c7ddb888339901524708472a3a/templates/repo/diff/box.tmpl#L90-L92 We can see that the compare box won't display if the file is marked as IsRenamed. So in which case this is likely a really simple fix - we just need to figure out what happens if we drop that IsRenamed test and if that works fine when there is no diff then boom we're done. --- So the above is just dealing with the header bar. We need to fix here to show the diff: https://github.com/go-gitea/gitea/blob/05b7e328297142c7ddb888339901524708472a3a/templates/repo/diff/box.tmpl#L116
Author
Owner

@joseluisq commented on GitHub (Apr 8, 2021):

Great, you got it.
BTW testing on try.gitea.io is I guess no longer necessary ?

@joseluisq commented on GitHub (Apr 8, 2021): Great, you got it. BTW testing on try.gitea.io is I guess no longer necessary ?
Author
Owner

@zeripath commented on GitHub (Apr 8, 2021):

Well if you have a simple reproducible example that would always be helpful.

@zeripath commented on GitHub (Apr 8, 2021): Well if you have a simple reproducible example that would always be helpful.
Author
Owner

@joseluisq commented on GitHub (Apr 8, 2021):

Ok, I will not bother you more I will give it a test.

@joseluisq commented on GitHub (Apr 8, 2021): Ok, I will not bother you more I will give it a test.
Author
Owner

@zeripath commented on GitHub (Apr 8, 2021):

Try #15340 (for 1.13)

Either download the requisite template and stick it in the correct place or compile from that PR.

@zeripath commented on GitHub (Apr 8, 2021): Try #15340 (for 1.13) Either download the requisite template and stick it in the correct place or compile from that PR.
Author
Owner

@joseluisq commented on GitHub (Apr 8, 2021):

Ok, since you have just modified the diff template and for closer verification and I will use it as a custom template directly in my server.

@joseluisq commented on GitHub (Apr 8, 2021): Ok, since you have just modified the diff template and for closer verification and I will use it as a custom template directly in my server.
Author
Owner

@joseluisq commented on GitHub (Apr 8, 2021):

Confirmed, it shows now the renamed diff.
Tested with https://github.com/go-gitea/gitea/pull/15340 on Gitea 1.13.7

@joseluisq commented on GitHub (Apr 8, 2021): Confirmed, it shows now the renamed diff. Tested with https://github.com/go-gitea/gitea/pull/15340 on Gitea 1.13.7 <img width="400" src="https://user-images.githubusercontent.com/1700322/114002034-cc7ba300-985c-11eb-80d0-9d206c004ce3.png">
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/gitea#7132