Closed MaximeBaudette closed 8 years ago
@geofjamg I'd prefer someone else takes a look before integrating the changes, that's why I went for the pull request instead of directly pushing to the repository.
When you merge it, I'll have to do the following:
There is indeed a problem with the changes. You added:
revisions="<!--DISCLAIMER-->
<html>
....
</html>
The problem is that your HTML comment element <!--DISCLAIMER-->
now sits OUTSIDE the HTML part of the revision. Which means it contains now a text part and an HTML part. This is bound to lead to problems for the display.
Better change it to:
revisions="<html>
<!--DISCLAIMER-->
....
</html>
I.e., start the revision string right away with the <html>
identifier.
Qed:
model Unnamed
annotation (Documentation(revisions="<!--DISCLAIMER-->
<html>
Foo
</html>"));
end Unnamed;
vs the correct version:
model Unnamed
annotation (Documentation(revisions="<html>
<!--DISCLAIMER-->
Foo
</html>"));
end Unnamed;
@geofjamg ok... so after @dietmarw 's comments I'll do some changes... but right now I wanna sleep so I'll take it tomorrow morning.
In the mean time, NO Merging.
@dietmarw I tried
revisions="<html>
<!--DISCLAIMER-->
....
</html>
But Dymola removed the , that's why I didn't do it... but now I tested again, and it's just that if you happen to edit the HTML via the WYSIWYG editor, then it removes it, doesn't matter where it is placed... So in short, I'll move it as you suggest.
\ Bonus Question**: Do you know if there is a way of blocking the HTML editor of Dymola?
I thought you wanted to go to sleep? ;-) Anyway I can confirm that Dymola removes the HTML comment. I've reported a ticket on this. My standard advice stands. Hands off of Dymola's WYSIWYG editor!
By editing all those files I guess you might have appreciated my more sophisticated solution (which would probably have cost you less time). I.e., a script that replaces the place holder <!--DISCLAIMER-->
by the actual disclaimer. If interested there are a whole set of nice helper scripts which can be adapted to your need in https://github.com/lbl-srg/modelica-buildings/tree/master/bin
Anyway since you do not yet use the place holder then you do not need to have it present. My solution I suggested was have either (and preferable) the disclaimer placeholder (and only that one) in the source and add the actual text via running a script for the released version only or add the disclaimer in all those places manually. The latter you've seen is quite cumbersome and in addition also prone to have duplicate errors and differences and is a nightmare to keep up to date.
Having said that, I have actually a much better idea which also makes use of Modelica's inheritance features and is easy to maintain :-) Give me an hour and I'll send a PR against master and let me know what you think.
@dietmarw Actually I took inspiration of this this place holder by keeping it even when the copyright info is present. Since I usually do as follows when updating the license info:
Given this workflow I can easily keep the placeholder to identify where to update the license. And also... now I have done the job of putting it everywhere (which was cumbersome), but we don't need to do it anymore.
Well you need to update it at least once every year. BTW we have 2016 now :smile:
yes, I was wondering about that 2016... :smile: I'll add it as 2015-2016
Well wait for my PR and maybe you like that solution better.
No problem ! I'm just preparing our "for the time being" alternative. Then I'll email the other collaborators to discuss with them. It will be much easier to discuss the two alternatives if I can show how things are done in the modelica code. And also I can explain what are pros and cons of both alternatives. So when you send your PR, could quickly describe what you see as pros and cons, so I can grasp it
@dietmarw Ok, now I took all your comments into account... Took me much longer than I thought as I ran into an encoding issue and was missing the escaping characters. I'm very curious about your PR to come :smile:
The copyright statement and license info is now moves in the revision part starting with
to identify the section closes #7, closes #62