- User Since
- Jul 30 2015, 8:46 PM (128 w, 6 d)
Sat, Jan 13
Tue, Jan 9
;=) I assume then a KDevelop person must try it.
Sat, Jan 6
Fri, Dec 22
If someone come up with a better solution, I'm ready to continue,
otherwise, ready to go ?
I think this is good enough ATM, lets see what people think of it.
Wed, Dec 20
And please add yourself to the copyright headers, I will need full name + email anyways for the commit.
If you would remove that, you would first need to write code that does the download via KIO and stuff.
I think that would be overkill, normally you want to work with a local file.
Just let that check be like it is.
Dec 18 2017
To me it still makes sense (shorter code, simpler to maintain, for the very unlikely event)
It would have been nice to explain why (documentation ?) you are so confident
that a file without any valid url
has for sure a valid display (for the error message) nonetheless.
The 5 as size is still bit strange, but as we have no end enum value, which we don't want to be BC, I guess that is no issue.
I like the code better with the array ;=)
I would use the enum values instead of 5 or 2, otherwise this is fine for me.
But then a "Not a local file" error could be shown, when it is actually the url that would be invalid.
So it was left aside; or please clarify ?
I think that would be just fine enough.
A non-valid URL is no local file (and I doubt somebody will ever get one, beside "no" url).
But I can live with two different messages, as long as both are inline via KTextEditor::Message, it makes no sense that one is a messagebox.
Dec 12 2017
Because I thought that if the url itself was invalid, then things were probably really bad, and it was safer not to rely on the visibility of the document ?
I think the best is to skip that and just warn if no local file, we don't care why it is no one ;=)
Intermediate variables like document render the code much cleaner.
Are they OK ?
Dec 11 2017
Ok, thanks for fixing the compile!
Dec 10 2017
Hi, just found this task (after the announcement on community list).
For the error message: Two different errors would be nice.
Dec 8 2017
For the QLatin1String on the right side in some contructors: should it not be always QStringLiteral? Not that it will matter a lot.
I assume the regex are right ;=)
In principle I have no problem with this extension.
For the code: Could you change KateConsole::slotRun() to report some more meaningful error like "can only run local files".
And do we perhaps want some info message that hints that you should save your file if it has unsaved changes?
I think that extension idea is good.
The functions won't really fit to document or view and an editor object could be used for other global things, too, without further polluting the global namespace with free functions.
For the "KateMessageWidget *m_topMessageWidget" and Co. members: Would it make sense to have just an array of pointers with index == the matching MessagePosition enum value?
Well, the scripting commands are also available on the command line as commands, aren't they? So you could essentially call yourself.
How can we have infinite recursion? By calling a command that triggers again the script function we did call the command in? If that is the only problem, I see no issue, that can happen with normal function calls already, perhaps we could guard chained command calls inside executeCommand, thought (aka guard that not twice the same "command" string arrives via set)
Nov 21 2017
If I don't misread the current diff, all things got done.
Can you commit or shall I do so for you?
Nov 19 2017
Unfortunately I never finished the https://cgit.kde.org/clones/baloo/cullmann/tbaloo.git/ port :/
Even more unfortunately there are close to zero useful baloo commits either since that time.
Nov 16 2017
;) Yeah, I think the default behavior is ok as it is, you are right.
;=) Actually I wanted that kdevelop just opened the project in the current working dir, if any if I start it without arguments ;=)
But --project dir is already nice I assume.
I can live with that patch, thought I still think we should just rename the helper, but perhaps that breaks other things if people relied on the name.
If that change does the trick for you, feel free to push this.
This avoids any race-conditions we would introduce with the single shot variant.
Nov 14 2017
I can live with that ;=)
But still I think kioslave5 as name would make this not needed and be more in line with kded5/...
Doesn't the QStandardPaths call below find it?
No, as that will use searchPaths.
Yeah, that was done to be able to provide win/linux install bundles.
Actually would it make not more sense to suffix the kioslave with "5" or something like that like we did for the libs to avoids clashs?
Nov 13 2017
Please give my idea an try, if you don't find an obvious flaw in it.
Hmm, perhaps a different approach would fix the issue:
Feel free to commit.
Nov 12 2017
Given people seem to have tested this, I would be ok to let it go in now.
I guess we need some "user-test" for it anyways.
Nice, makes that plugin more usable for non-C++ things!
The single shots are not that nice, but I understand that they are necessary to defer the action.
Feel free to commit.
Hmm, isn't there some potential problem that the next call might use that untitled document again before the event loop call happens and we close it then?
Perhaps a better fix would be to defer the open call in the vi mode?
Nov 6 2017
Could you just remove the problematic includes and try to compile without the dependency, IMHO I see no use of QtScript stuff.
Hmm, I would rather like to have this fixed than having that dependency just for that.
I thought during the port it compiled completely without QtScript.
Nov 2 2017
I could live with that change.
It at least will avoid random crashs for the time being until we perhaps have a better solution.
Oct 31 2017
In Qt 5.6.0 it was already:
Before we got to excessive solutions, I assume you have a build were you can reproduce the crashs.
Hi, first: thanks for working on getting the crashs for older versions away.
Oct 25 2017
In the report, the user wanted some configurable directory.
I don't think this is something more than a few people want and not worth the effort.
The snippet tools stores the snippets in a user local config (if you add some as a user).
This would imho make most sense for me, to have in the snippet plugin some easy way to generate new ones from context menu or such, but I am not sure if there is not already the UI for that, as I never use the snippets stuff.
Perhaps you could just try out, what the snippet plugin provides.
Oct 23 2017
Hi, I am not sure this feature is that important to have.
Oct 17 2017
Then I would say => merge it.
And thanks for the work!
Oct 16 2017
Thanks for the fix!
Just one thing: could we in addition have an addition indentation test that uses the trigger chars? Its amazing we seem to have none for this as otherwise this problem would have been fixed during the port.
Oct 11 2017
All issues addressed, or?
Given it seems Atom even has the option to choose the sign for that, we can live with an option I assume (and the code for it is already written anyways).
Sep 24 2017
I would rather go with the "make the markers much larger but less contrast" solution than a setting for their size.
Perhaps we really just need to look at how other editors do that.
Eye-cancer like markers are no solution either, but I agree that if people turn them on, they should be able to spot them easily.
Sep 22 2017
Yes, that is ok.
Hoi, any feedback on this?
Sep 19 2017
I went over the diff, beside the few things not converted I see no obvious faults.
If nobody else objects, I would approve this.
Sep 18 2017
I still think we should just apply it, even if we change later again the shortcuts, it doesn't hurt and works around the issue for Kate.
Sep 17 2017
Given no feedback my temptation now wins over me. So preparing some independent release(s) now for the start.
Hi, please add a testcase and we are done here.
Without a test case this can easily degenerate later again.
I think you are right, the indexer is an other issue.
We could even just disable that index... files are used by the framework beside for its own internal dirs.
Sep 15 2017
In deed, looks correct now and bogus before.
Yeah, +1 ;=)
Without screeny, we can't judge.
In any case: thanks for taking care!
Looks reasonable, one consistency check more is always good!
(hehe, and even found already things)
Sep 7 2017
Btw, if qrcs overlay, then you can already now overlay org.kde.syntaxhighlighting/syntax and /theme :-)
QRC support is definitely preferred, but that should work with the original proposal indeed. Having just one fixed QRC extension path baked in would be problematic for PIM with its hundreds of libs/components for example, AFAIK QRCs don't overlay on the same path, do they?
Sep 6 2017
That would also be an option. Note, though, that the current patch also would allow that: repository.addCustomSearchPath(":/my-app-resource"); So with this API, you have both, folders on disk, and support for custom resource files.
That is true, the question is if you need that flexibility.
If we just standardize some resources dirs for that, applications need zero api calls and just can package the wanted files in the right dirs and be done.
Hmm, I think the idea is good.
Should be fixed now:
I see, the ref file is missing in autotests/reference
Sep 5 2017
syntax-highlighting.git, on first glance not landed atm.
Aug 28 2017
What would be your motivation for this? In other words, what use-cases would that solve for you?
Aug 27 2017
Hmm, I think it would be needed to e.g. ensure sysadmin has backup of the non-kde infrastructure hosted parts like the website.
And normally one would create a falkon-... mailing list as contact point and a bugzilla falkon product.
Aug 26 2017
I would have no problem to move out "all" plugins to some ktexteditor-addons repo.
Looks ok for me.
Aug 24 2017
Yeah, we want to keep that ;=)
Aug 22 2017
Three things I see ATM:
Aug 21 2017
Ok, I am happy with that ;=)
Thats kind of cool and given its even only very little code and a plugin I would like to have it, if others don't disagree.
Hmm, the diff still looks like it has the old names, perhaps the update went wrong somehow?