Various improvements

Personally , unless their is a very good reason why we should, I dont think we should replace the argument selection in "Improvement 2", but maybe add as an alternative so as not to cause extra work for websites that have implimented the existing method. I myself will have to fix the content of over 1000 pages

I like the improvements list. Haven't had a chance to test the patch yet.

Like midkemia, I think the argument format change has the danger of being very disruptive for existing sites. I'd support the change if we had legacy handling for the old argument type, but then, we're perhaps adding complexity and change without a strong need for change. Any other users want to weigh in here?

Perhaps the best course of action would be to move #2 to its own issue, where it will have an opportunity to get more attention and comments in the issue queue, and then we can proceed here with reviewing 1, 3, 4, and 5.

The old argument format still works. I just added a new one..

this is the complexity of legacy handling:

The panels module uses this argument format and quicktabs module switched to this format recently #382108: Separate view args with / rather than comma. This format is easy as it is the same what is used inside views admin interface for the preview..

The possible problem with the old format is that comma can be used to separate taxonomy terms for one argument, like arg1/term1,term2,term3/arg3/arg4, but of course this is the same as using arg1/term1+term2+term3/arg3/arg4

@Pasqualle

The complexity I was thinking of had more to do with confusion and support requests in the issue queue. :D

As long as the README.txt and the project page here on d.o are updated to reflect these changes, I don't see there being a problem with proceeding once we have a review or two of the current patch.

I tried using the patched module, but my insert views simply stopped inserting (which is a pity, as I thought it looked like a good improvement). Not sure why they stopped working yet.

maybe you do not have access for the view. can you check the access set for the view?

or can you write the Insert View tag used to display the view? do you have spaces in it, or anything special?

arg1/term1,term2,term3/arg3/arg4, but of course this is the same as using arg1/term1+term2+term3/arg3/arg4

it seems like this is not true

from the issue #430596: Changing Argument Separator
the comma is used as OR
and + is used as AND

so the separator issue should be solved only in the 2.x version

Could improvement #2 be made a configurable option? I support changing the argument separator to match views.

@Digital Deployment: yes, that could be an option.. I think that's a good idea, as I do not know any other possibility how to fully support the upgrade from the old version..

Returning to my initial concern #1

I dont know exactly how updgrading works and what it can do, or is allowed. But the biggest issue I see stopping wholesale replacement of the old technique is identifying and making the changes which could be a big task.

So
1/is there a way of of identifying all nodes that have "Insert_view" used probably look for "[view:"

2/ if there is, is there a way of providing a scrpit that is run after update of the module, that will run through all affected nodes making the appropriate changes to the new format probably after finding "[view:" substitute the appropriate items before the next "]"

In theory this would simplify updates for all, and with a change point of 2.x, anyone using that onwards will have the new technique

1. I would not run an automatic update on node content. That could really mess up things in special cases.
2. The filter can be used on various places, like it can be used in blocks, I even used the filter format in theme templates (now I am using the insert_view() function).

I can see the potentail risk from what you say.

An option to identify all locations where permissible, would possibly be a benefit, as i can see on a larger sites, rembering where insert view has been used could be an issue. This might also be a useful permanent feature

As an option, you can create a view of nodes to find [view: in the node body so that is an option for those using views using [view:%,

Not sure about this as I am pretty new to Drupal but could the module support 2 tag types.

At the moment all tags begin [view:......

ie: [view:name of view=x=arg1,arg2,arg3]

What about another tag that begins [view2:....

[view2:name of view=x=arg1/arg2/arg3] (Using new argument format)

Andy

The way i understand it , is the intent was to bring the module into line with the argument format used by views, so ideally replacing all exisiting cases would be best.

What was being offered was a means to permit old format to remain, but if they're changed , the need to support goes away

Can I please suggest adding support for escaping the argument separator? There's a simple example at http://drupal.org/node/85771#comment-158116 .

Re #15: The patch already has a more commonly used separator '/'. You should write the arguments same as you write them on the views admin interface at preview.

Doesn't this code mean that ',' are replaced by '/'?

So now if your argument contains either character then it will be treated as multiple arguments? This means that adding an extra separator character makes the problem worse rather than better because you now have 2 characters that can't be used in an argument. :-)

The views preview interface seems to have a similar problem.

For example, say that my view displays nodes of a particular content type whose node title matches a single argument. If I have a node with title "A, B" then I can't currently match it using insert_view 6.x-1.0. In your proposed version I still can't use insert_view to match that node and I also can't match a node with title "X/Y".

You need a way of telling insert_view that a ',' or '/' should be treated as a character that is part of an argument instead of a character that is used to split multiple arguments. One way of doing this would be to add an escape character like the example I linked to above.

Another way would be to make the argument separator configurable per use of insert_view. The cleanest way I can think of doing this would be something like this example:

[view:@my_view=default=arg1@arg2]

That is, if you put a character that isn't allowed in a view name at the beginning of the view name, then that character is used as the argument separator. I haven't though about how this might interact with the PHP filter if you allowed it in the same input filter as the insert_view filter... but if you're allowing PHP then you probably don't need insert_view. ;-)

That code snippet is wrong and should be removed.. It was meant to be a backward compatibility code, but it does not work correctly if your argument does not contain a slash..

this works correctly even with that code

but this:

does not. As the code do not know if you want to use multiple values for 1 argument or 3 arguments..

the views preview interface works, although I do not know if your example (/ inside node title) is handled correctly. The argument separator is '/' (slash), the separator for multiple values for the same argument are: ',' (comma) or '+' (plus) depending what is the relation between the values:
, equals OR
+ equal AND

we do not want to invent a new separator, we want to use the same separators as views does. That is the cleanest way.. If you find a problem with these separators you should open an issue for views..

The views preview interface has a similar problem but the problems are independent.

When insert_view does this

it splits on every '/' in the argument(s). There is no way to include a '/' in an argument, so you can't possibly pass an argument that matches a field that has a value containing '/'. To do this you would need to be able to escape a slash. For example, arg1part1\/arg1part2/arg2.

but come on Martin, that is how we use arguments in Drupal.
http://api.drupal.org/api/function/arg/6
It is not insert_view's fault that you can't use / inside arguments..

I partly agree. So, here's my last throw of the dice... :-)

insert_view parses the arguments into an array so it has the opportunity to decide how that parsing should be done.

If I resort to PHP and call views_embed_view() directly then I can pass arguments containing any characters, because those arguments are not parsed. So the problem is not inherent in views.

It makes sense to allow people to pass any string as an argument, even if that string contains a metacharacter. Most software allows metacharacters to be escaped.

Perhaps the real solution is to use URL encoding. In that case, perhaps views needs to decode such encoded characters in arguments before building the query? Perhaps the decoding should be optional per argument?

you are right about views_embed_view(), you can put a / inside argument..

but any page or block view can not accept such arguments, so just to make it possible with embedded views we should make the arguments even more complicated. This argument handling code was based on the code in panels 2, and it is quite bulletproof.

I've applied the patch and the module has stopped working altogether, instead outputting clear text on page view:

[view:emerg_contacts_lerp_display=page=Facilities & Services Division]

The argument type is set in my view to "Term name/synonym converted to Term ID"

I know there is a problem, that the filter text is displayed when the view is empty.
Can you check that the view is not empty? Just try the preview in the view admin interface with the "Facilities & Services Division" argument ..

Hi Pasqualle,

This is on a page that shows the view inline correctly, until I apply the patched version of the module, if I revert back the view is displaying inline correctly again.

So there are definitely records that get returned with the argument.

Subscribing.

I find it to be a pretty useful module however the documentation is pretty confusing and so is the example given in the read me text. All I need to do is pass on an argument using the url . However I am not able to figure out how ..

The syntax i m using : [view:tracker=page=1] works when "1" here is the value of the argument being passed. However I want to pass the argument "user ID" dynamically which ideally should be taken from the url and not supplied as value. Please help.. Any suggestions would be appreciated.