Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SetHyperlink in the second sheet causing file format errors. #14

Closed
erdian718 opened this issue May 4, 2019 · 17 comments
Closed

SetHyperlink in the second sheet causing file format errors. #14

erdian718 opened this issue May 4, 2019 · 17 comments
Labels

Comments

@erdian718
Copy link

demo.xlsx

SetHyperlink in the first sheet works fine, but not fine in the second sheet.

Open with MS Office 2016:

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<recoveryLog xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main">
	<logFileName>error208200_01.xml</logFileName>
	<summary>在文件“C:\msys64\home\ofunc_7kwo01e\go\src\test\xlsx\demo.xlsx”中检测到错误</summary>
	<additionalInfo>
		<info>Excel 已完成文件级验证和修复。此工作簿的某些部分可能已被修复或丢弃。</info>
	</additionalInfo>
	<repairedParts>
		<repairedPart>已修复的部件: 部件 /xl/worksheets/sheet2.xml。</repairedPart>
	</repairedParts>
</recoveryLog>
@plandem
Copy link
Owner

plandem commented May 4, 2019

I checked your demo file and see the reason. Did you create that file using library? Because I see some Excel things. I need to know how to repeat issue - i tried to create file using library and could add hyperlinks in both sheets

@plandem
Copy link
Owner

plandem commented May 5, 2019

btw, I added conditional formatting feature. Looks like everything is good, but maybe more validation and other advanced settings required.

You can check it this branch: https://github.com/plandem/xlsx/tree/feature/conditional

P.S.: Sadly, but I did small api change that not compatible with previous version - format.New was renamed to format.NewStyles, because right now it's also possible to use format.NewConditions

@erdian718
Copy link
Author

The file demo.xlsx works fine with WPS, but not fine with MS Office.
I will upload the original file and the test codes later (maybe tomorrow), they are in my another computer, which has MS Office installed.

@plandem
Copy link
Owner

plandem commented May 5, 2019

Would be cool, because I don't have any issues with links at different sheets. So I need steps to reproduce - code.

@erdian718
Copy link
Author

package main

import (
	"github.com/plandem/xlsx"
)

func main() {
	xl, err := xlsx.Open("test.xlsx")
	if err != nil {
		panic(err)
	}
	defer xl.Close()

	sheet := xl.Sheet(0)
	sheet.CellByRef("G1").SetValueWithHyperlink("LINK测试1", "#C3")
	sheet.CellByRef("G2").SetValueWithHyperlink("LINK测试2", "#'Sheet1'!C3")
	sheet.CellByRef("G3").SetValueWithHyperlink("LINK测试3", "http://baidu.com")
	sheet.CellByRef("G4").SetValueWithHyperlink("LINK测试4", "spam@spam.it")

	xl.SaveAs("out.xlsx")
}

test.xlsx

out.xlsx

@plandem
Copy link
Owner

plandem commented May 5, 2019

ok, I can reproduce with that test file, but I have a question - how did you get that original file? Because I see messed sheet's id first time - did you created this file(original) using library?

@plandem
Copy link
Owner

plandem commented May 5, 2019

    <sheets>
        <sheet name="Sheet2" sheetId="3" r:id="rId1"/>
        <sheet name="表1" sheetId="1" r:id="rId2"/>
        <sheet name="Sheet1" sheetId="2" r:id="rId3"/>
    </sheets>

normally sheetId related to r:id

@plandem
Copy link
Owner

plandem commented May 5, 2019

ok, I found a bug. It was actually very critical bug, but luckily that functionality added only with hyperlinks feature. Right now, I pushed fix into conditional branch only.

@plandem plandem closed this as completed May 5, 2019
@plandem plandem added the bug label May 5, 2019
@plandem
Copy link
Owner

plandem commented May 5, 2019

btw, what to do you think about overall usage - is it friendly? I want to make library that will be easy to use and possible to rely on IDEs code completion feature - no need to look for xlsx documentation, IDE will suggest possible value.

Right now I'm double minds about
a) few packages only and use it as right now, more typing, backward compatibility:
types.Hyperlink.NewHyperlink,
types.Hyperlink.ToFile,
format.NewStyles,
format.NewConditions

b) dedicated packages and better readability as result, broken backward compatibility:
hyperlink.New,
hyperlink.ToFile,
styles.New,
conditions.New

I like later version more, but in that case more imports and broken backward compatibility

@plandem
Copy link
Owner

plandem commented May 5, 2019

I refactored packages as, you can check here:
https://github.com/plandem/xlsx/tree/refactor-packages

but as I said no backward compatibility, although I like that way more. I will keep it as is right now, need to sleep more with that way.

@erdian718
Copy link
Author

erdian718 commented May 6, 2019

I used tealeg and excelize for a while, but none of them works as well as your xlsx.
I have made some data reports using your xlsx, and they are working well at present.
I also like later version more. Backward compatibility is not a big problem. After all, 1.0 version is not released, some incompatible changes are acceptable.

@plandem
Copy link
Owner

plandem commented May 6, 2019

I tried both too and while I liked tealeg at first sight, it could not process well formatted files - always corrupted after saving and in my case it was not generated xlsx, but used by clients every day as dumb "database" replacement with notes, formatting and etc.

excelize was better in some cases, but also could not process and I had to patch non stop that library and at some point I caught myself that I spend too much time to dive into very ugly and bad spagetti code(at least that time, not sure right now), so I dropped that idea and thats how that lib was born.

P.S.: I like later version more and like it waaaay better, so many areas to simplify even right now - improve styles/conditionals much more. Will try to finish it this week. Btw, conditional finished - full support right now, but probably need more decent validation for rules, because it's easy to create invalid settings.

@plandem
Copy link
Owner

plandem commented May 7, 2019

ok, so after trying to use conditional, I caught myself that I still need to check XLSX documentation how to set proper value and that not what I want from library. So I started to refactor in more human friendly way, but right now its very limited. Usage will be like this:

import (
	"fmt"
	"github.com/plandem/xlsx"
	"github.com/plandem/xlsx/format/conditional"
	"github.com/plandem/xlsx/format/conditional/rule"
)

func main() {
	xl := xlsx.New()
	defer xl.Close()

	s := xl.AddSheet("The first sheet")
	s.CellByRef("A1").SetValue(10)
	s.CellByRef("A2").SetValue(20)
	s.CellByRef("A3").SetValue(30)
	s.CellByRef("A4").SetValue(40)
	s.CellByRef("A5").SetValue(50)
	s.CellByRef("A6").SetValue(60)
	s.CellByRef("A7").SetValue(70)
	s.CellByRef("A8").SetValue(80)
	s.CellByRef("A9").SetValue(90)
	s.CellByRef("A10").SetValue(100)

	err := s.AddConditional(conditional.New(
		conditional.AddRule(
			rule.DataBar.Default,
		),
		conditional.AddRule(
			rule.DataBar.Min("100", rule.ValueTypeNumber),
			rule.DataBar.Color("#FF00FF"),
		),
		conditional.AddRule(
			rule.ColorScale2.Default,
		),
		conditional.AddRule(
			rule.ColorScale3.Min("1", "#FF0000"),
			rule.ColorScale3.Mid("50", "#FFFF00"),
			rule.ColorScale3.Max("10", "#00FF00"),
		),

	), "A1:A10")
	fmt.Println(err)

	if err := xl.SaveAs("./test_files/first.xlsx"); err != nil {
		fmt.Printf("Error saving document to file: %s\n", err.Error())
	}
}

@plandem
Copy link
Owner

plandem commented May 7, 2019

you can check current version at https://github.com/plandem/xlsx/tree/refactor-packages

@erdian718
Copy link
Author

erdian718 commented May 8, 2019

I updated the ooxml and xlsx code today, but build failed in the master branch: ml.OptionalIndex is not a type.

@plandem
Copy link
Owner

plandem commented May 8, 2019

yes, due to breaking backward compatibility, I had to release as 1.0.0 both packages. ooxml has version too right now

@plandem
Copy link
Owner

plandem commented May 8, 2019

ok, should work fine right now, although I removed 1.0.0 from xlsx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants