-
-
Notifications
You must be signed in to change notification settings - Fork 580
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
Support OSC-SCM #1890
base: master
Are you sure you want to change the base?
Support OSC-SCM #1890
Conversation
Tests are not mandatory, but if you want to add some you'll need to basically create a dummy project in which to run the project detection commands. There are plenty of similar tests already. I'll have to take a closer look at the code, but overall the PR seems reasonable to me. |
@@ -43,6 +43,7 @@ | |||
(require 'compile) | |||
(require 'grep) | |||
(require 'lisp-mnt) | |||
(require 'xml) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest moving this in the function that uses it, so we don't slow down loading Projectile needlessly.
@@ -767,6 +771,10 @@ Set to nil to disable listing submodules contents." | |||
"Command used by projectile to get the files in a svn project." | |||
:group 'projectile | |||
:type 'string) | |||
(defcustom projectile-osc-command #'projectile-osc-command-files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a blank line before this.
(defcustom projectile-osc-command #'projectile-osc-command-files | ||
"Command used by projectile to get the files in a svn project." | ||
:group 'projectile | ||
:type '(function string)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't forget to add a :package-version property.
(defun projectile-osc-command-files (&optional root) | ||
"Return files of osc vcs, return either packages or files belonging to package." | ||
(or root (setq root default-directory)) | ||
(let* ((files_ (car (xml-parse-file (if (file-exists-p ".osc/_files") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why files_
instead of files
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to me a cond
might be a better fit than the nested if
s you've opted for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why
files_
instead offiles
?
Because the file containing the content is called _files
and _files
as variable name didn't exactly work.
(let ((default-directory root)) | ||
(if (functionp command) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm puzzled by this code, as you call the command, but don't do anything with the result.
Signed-off-by: Björn Bidar <[email protected]>
Let me know when you feel this is ready for another review. |
a700dd9
to
8d24fac
Compare
Support the OSC SCM as used in the open build service.
OSC can be used with vcs-osc in Emacs.
The code is wip but I wanted to post this PR to get some feedback
and show progress working on it.
Osc works a little different than other scms where there two different
types of repositories, one containing packages and the other being a package.
The type that is package works very much like a regular scm where commands
such as those to open project files work as a user would expect.
But for the project type the commands referring to files target the packages the project
contains.
I've implemented the functionality using the builtin XML functionality as that is the easiest
since osc's own files use XML.
I'm not exactly sure how to add test, any help is appreciated.
Before submitting a PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):
eldev test
)M-x checkdoc
warnings