Commit Details

[antis81]

• [x] Commit detail view should be based on QWebView
• [x] Add a great style
• [x] Replace certain CSS attributes with values from MGV settings
• [ ] Polish the style :gem:

[scunz]

The code looks nice; the comments are actually just "free standing thoughts" about how to do this in the future. Please go ahead with this ^^

[antis81]

@scunz How about merging this one and talking about sass later? Btw. it would support also variable defaults :smile:.

[scunz]

We should not aim for such a high level solution. Replacing a few variables in the file ought to be sufficient, shouldn't it?

[antis81]

Hm.. ok, just thought if this could help in several places, but I think you're right. Btw. I just started a new branch with "new" features in MGV-Modules, seems your work starts to become visible :).

[antis81]

Rebased onto development with new RM changes.

[antis81]

@scunz Review, please :smile:.

[scunz]

@antis81, first of all, I would really appreciate if this could be made a globally accessible feature. Meaning: We should not bury it deep down in a module, but rather find a nice place in libMacGitverCore.

Second, I'm not sure what the additional paren symbols are about. $(xxx)$ looks at least unfamiliar. Why not stick to simple bash style macros. In the logging code, I use Windows cmd.exe styled macros but with the usual dollar sign known from unix ($FOO$). However, when I look at your code, it seems to simply assume any undefined macros as being empty. Actually, I think this is a good idea (and kind of rectifies the parens). However, I would still prefer to have something bashy. What about the bash default: ${FOO} with braces instead of parens and no tailing dollar?

Actually, I would prefer if we could create a common thing inside the core that could be used by both the logging code and the weclcome and history modules.

Further, I think that it might be a good idea to limit the length of a macro's name to at most, say, 32 characters. Maybe some document contains a stray ${ and a macro after that...

As for the style/html itself: we still need to polish it a bit. Tried this code recently and when scrolling through the history, the header entries in the history table kept jumping horizontally. The cells should be top-left aligned. As we're trying to make everything look a little smoother: do we want gradients and rounded corners here?

[antis81]

What about the bash default: ${FOO} with braces instead of parens and no tailing dollar?

I agree, but would rather like to stick to the more simple syntax used by sass and bash also. I think we can handle $FOO variables followed either by a semicolon or a word boundary (space, crlf, ...). Also thought about the additional length limit, which is good enough for our purpose and make it a little more "solid".

Actually, I would prefer if we could create a common thing inside the core that could be used by both the logging code and the weclcome and history modules.

Again, I wish for the same thing. We can reuse the mechanism btw. for the Custom Commands. In this case, we can merge (after the syntax cleanup mentioned above) and enhance this in mgv-core. Further the matches list can be optimized a little by adding each entry only once (which is also a potential failure in the foreach loop).

As for the style/html itself: we still need to polish it a bit.

Yep, also noticed that. In addition, the "long message" has a fixed width and the table cannot be resized smaller than x. Added another todo to that above (see :top:). For the gradients I suggest to play around with 'em and provide some screenhots. The basic idea was to use the "heading" gradient I used in the ref-tree. I even thought about making it full width without margins and borders.

[scunz]

I agree, but would rather like to stick to the more simple syntax used by sass and bash also. I think we can handle $FOO variables followed either by a semicolon or a word boundary (space, crlf, ...). Also thought about the additional length limit, which is good enough for our purpose and make it a little more "solid".

Well, now we're talking about downsizing the feature.

We can reuse the mechanism btw. for the Custom Commands. In this case, we can merge (after the syntax cleanup mentioned above) and enhance this in mgv-core.

And here, we're introducing new requirements that require upsizing the feature...

Actually, I think that the Custom Commands Idea is pretty good. But it requires us to do this thing once and to do it right. Well, there's more than one reason, bash has both the $foo and the ${foo} syntax. If we go that route, let's kick off the RegEx crap and let me write a decent parser that will make this and some other nice features work...

I'd suggest: Stick with the $foo syntax as long as it serves your purpose well. Just introduce a place somewhere down in the core, where we have a method with the signature QString expandMacros(const QString& input, const QHash<QString, QString>& macros). Don't implement this to deeply, as I will scratch it away with something much more sophisticated[¹]. A loop of simple calls to QString::replace("$foo", "bar") should do it for now.

[¹] actually, I'm now thinking about to fully support bash style string replacements...

[scunz]

Btw.: Even with the matches-optimization you refer to, this can badly (and esp. randomly) be used to fuck up everything: Given the template: $(A)$ $(B)$ $($(C)$)$ and the parameters A=$(B)$ and B=$(A)$ and C=B what will be the output?

Now, change the template to $($(C)$)$ $(B)$ $(A)$ and try again :)

[antis81]

Sorry for the late reply. I'll add the central string replace in mgv core like you stated above. Then I think of writing the parser as it's been loong time I did such things and this provides good practice for me :smile:.

[scunz]

Okay, here's a subset of the bash parameter expansion that I think is worth to support. In all of these, parameter is subject to the replacement logic itself.

• ${parameter} or $parameter (Value get) return value of parameter.
• ${parameter:-word} (Use default value) if parameter is empty, call replacement logic on word and return the result. Otherwise return parameter.
• ${parameter:=word} (Use and assign default value) if parameter is empty, call replacement logic on word and store the result in parameter. Then return parameter.
• ${parameter:+word} (Use alternate value) if parameter is empty or unset, return empty. Otherwise, apply replacement logic to word and return its result.
• ${parameter:offset:length} or ${parameter:offset} (Substring) Return a substring of parameter starting at offset. If length is given, limit to that amount of characters.
• ${#parameter} (Length of) Return the number of characters in parameter as a string.
• ${parameter/pattern/string} (Replacement) Replace all occurrences of pattern in parameter with string. Return the result. string is subject to the replacement logic.
• ${parameter##word} and ${parameter#word} (Prefix removal) word is subject to the replacement logic. If the beginning of parameter matches word, the shortest (#) or longest (##) match is removed from parameter and the result returned.
• ${parameter%%word} and ${parameter%word} (Suffix removal) word is subject to the replacement logic. If the tail of parameter matches word, the shortest (%) or longest (%%) match is removed from parameter and the result returned.

Any matching here refers to the (actually simplified) path name expansion method of bash:

• ? means any one character
• * means any number of characters
• [a-cg] means a, b, c or g
• [^abc] means any character but a, b or c

Having that said, I can't await to see it implemented. I think, the "Pathname expansion", "Replacement logic" and "Parameter expansion" should all be separate classes. Actually, I even think that these would be worth for a library on their own :)

[antis81]

The latest commits together with the css font fix PR in libMacGitverCore give better results :).

Some more polishment questions and topics:

• The first column of the detail table should be fixed size, right?
• In general, the fonts used here are very big compared to everything else. Should be more consistent.
• Is it possible to set the minimum width of each other box to table width? Currently, when horizontal scrollbar appears, things look a bit crappy when scrolling to the right (missing background etc.).

[antis81]

Think, also not 100% ready, this can as well be merged. Everything here is definitely better that before :).

[scunz]

just go ahead. But seems this needs rebasing or something; merge button is disabled...

[antis81]

Merged and fixed manually. Really interesting: We added both a rcc file with different names. Some minor conflicts only in CMakeLists.txt. Everything works fine :).